From 26d693d85e2be7d9129308863fdcadce628e99e9 Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Tue, 18 Aug 2026 19:09:50 +0500 Subject: [PATCH] sd: extend Undo to cover the placeholder mechanics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting all the text of a placeholder was not undoable: sd puts the prompt back before svx captures the text change, so the action it creates sees the prompt as both the old and the new text and drops itself. The text is now recorded before the prompt goes in, in one group with the change of what represents a picture placeholder, so one step takes the whole deletion back. Two things that only surface through that step: a kind with no style sheet of its own kept none and painted itself in the pool default fill colour, and the placeholder built to stand in for the text recorded an insert that redo performed a second time, on top of the swap. Signed-off-by: Mike Kaganski Change-Id: I7bd581216b9111197144d4691c2a12c996cf0e3f Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/9442 Tested-by: Jenkins CPCI Reviewed-by: Caolán McNamara (cherry picked from commit 33b95acb2e26a22d3242605f7d609108a678620e) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210330 Tested-by: Jenkins Reviewed-by: Xisco Fauli --- sd/qa/unit/import-tests2.cxx | 55 ++++++++++++++++++++++++++++++++++++ sd/qa/unit/uiimpress.cxx | 33 ++++++++++++++++++++++ sd/source/core/sdpage.cxx | 21 +++++++++++++- sd/source/ui/view/sdview.cxx | 29 +++++++++++++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) diff --git a/sd/qa/unit/import-tests2.cxx b/sd/qa/unit/import-tests2.cxx index 04a804360532..c9235bf57282 100644 --- a/sd/qa/unit/import-tests2.cxx +++ b/sd/qa/unit/import-tests2.cxx @@ -1517,12 +1517,43 @@ CPPUNIT_TEST_FIXTURE(SdImportTest2, testTdf166401_typingSwitchesAnEmptyPlacehold xText.queryThrow() ->getPropertyValue(u"PlaceholderShapeType"_ustr) .get()); + + // One undo step takes the typing back, and the placeholder waits for a picture again. + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); + auto xUndone = getShapeFromPage(0, 0).queryThrow(); + CPPUNIT_ASSERT_EQUAL(u"com.sun.star.presentation.GraphicObjectShape"_ustr, + xUndone->getShapeType()); + bool bEmptyAgain = false; + CPPUNIT_ASSERT(xUndone.queryThrow()->getPropertyValue( + u"IsEmptyPresentationObject"_ustr) + >>= bEmptyAgain); + CPPUNIT_ASSERT_MESSAGE("the placeholder kept the typing", bEmptyAgain); + + // The identity has to be live after undo too, not the shape type the UNO shape cached when it + // was created. + CPPUNIT_ASSERT_EQUAL_MESSAGE("the placeholder identity did not survive undo", + u"com.sun.star.presentation.GraphicObjectShape"_ustr, + xUndone.queryThrow() + ->getPropertyValue(u"PlaceholderShapeType"_ustr) + .get()); } CPPUNIT_TEST_FIXTURE(SdImportTest2, testTdf166401_emptyingHandsBackTheIdentity) { createSdImpressDoc("pptx/pic-placeholder-with-text.pptx"); + auto styleNameOf = [](const uno::Reference& xShape) -> OUString { + SdrObject* pShapeObj + = SdrObject::getSdrObjectFromXShape(xShape.queryThrow()); + SfxStyleSheet* pStyle = pShapeObj ? pShapeObj->GetStyleSheet() : nullptr; + return pStyle ? pStyle->GetName() : u""_ustr; + }; + const drawing::FillStyle eFillBefore + = getShapeFromPage(0, 0)->getPropertyValue(u"FillStyle"_ustr).get(); + const sal_Int32 nFillColorBefore + = getShapeFromPage(0, 0)->getPropertyValue(u"FillColor"_ustr).get(); + const OUString aStyleBefore = styleNameOf(getShapeFromPage(0, 0)); + // Deleting the whole text hands the placeholder its identity back: it shows what it waits for. sd::ViewShell* pViewShell = getSdDocShell()->GetViewShell(); sd::View* pView = pViewShell->GetView(); @@ -1552,6 +1583,30 @@ CPPUNIT_TEST_FIXTURE(SdImportTest2, testTdf166401_emptyingHandsBackTheIdentity) CPPUNIT_ASSERT(pBack->GetOutlinerParaObject()); CPPUNIT_ASSERT_EQUAL(u"Custom prompt to insert an image"_ustr, pBack->GetOutlinerParaObject()->GetTextObject().GetText(0)); + + // One undo step takes back the whole deletion: the text is there again, held by the object that + // represents a placeholder holding text. + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); + auto xUndone = getShapeFromPage(0, 0).queryThrow(); + CPPUNIT_ASSERT_EQUAL(u"com.sun.star.presentation.OutlinerShape"_ustr, xUndone->getShapeType()); + // It is the object it was, styles and all: left with no style of its own it would fall back to + // the pool defaults, which paint it in the default fill colour. + CPPUNIT_ASSERT_EQUAL( + eFillBefore, + getShapeFromPage(0, 0)->getPropertyValue(u"FillStyle"_ustr).get()); + CPPUNIT_ASSERT_EQUAL( + nFillColorBefore, + getShapeFromPage(0, 0)->getPropertyValue(u"FillColor"_ustr).get()); + CPPUNIT_ASSERT_EQUAL(aStyleBefore, styleNameOf(getShapeFromPage(0, 0))); + CPPUNIT_ASSERT_EQUAL(u"Typed into a picture placeholder"_ustr, + xUndone.queryThrow()->getString()); + + // And it is what the page says it is, not the shape type cached at creation. + CPPUNIT_ASSERT_EQUAL_MESSAGE("the placeholder identity did not survive undo", + u"com.sun.star.presentation.GraphicObjectShape"_ustr, + xUndone.queryThrow() + ->getPropertyValue(u"PlaceholderShapeType"_ustr) + .get()); } CPPUNIT_TEST_FIXTURE(SdImportTest2, testTdf166401_aPictureWithTextKeepsThePicture) diff --git a/sd/qa/unit/uiimpress.cxx b/sd/qa/unit/uiimpress.cxx index 7a7440e0b9a0..1335ed4f8fea 100644 --- a/sd/qa/unit/uiimpress.cxx +++ b/sd/qa/unit/uiimpress.cxx @@ -2563,6 +2563,39 @@ CPPUNIT_TEST_FIXTURE(SdUiImpressTest, testPasteInTextEditWithAnimationNode) CPPUNIT_ASSERT_EQUAL(u"blahblah"_ustr, rEditView.GetSelected()); } +CPPUNIT_TEST_FIXTURE(SdUiImpressTest, testUndoRestoresDeletedPlaceholderText) +{ + // Given a slide whose title placeholder was typed into: + createSdImpressDoc(); + auto* pXImpressDocument = dynamic_cast(mxComponent.get()); + sd::ViewShell* pViewShell = pXImpressDocument->GetDocShell()->GetViewShell(); + sd::View* pView = pViewShell->GetView(); + SdrObject* pTitle = pViewShell->GetActualPage()->GetObj(0); + CPPUNIT_ASSERT(pTitle); + pView->MarkObj(pTitle, pView->GetSdrPageView()); + + pView->SdrBeginTextEdit(pTitle); + pView->GetTextEditOutlinerView()->GetEditView().SetSelection(ESelection::All()); + pView->GetTextEditOutlinerView()->GetEditView().InsertText(u"Typed"_ustr); + pView->SdrEndTextEdit(); + CPPUNIT_ASSERT_EQUAL(u"Typed"_ustr, + pTitle->GetOutlinerParaObject()->GetTextObject().GetText(0)); + + // When deleting all of that text, which puts the placeholder's prompt back: + pView->SdrBeginTextEdit(pTitle); + pView->GetTextEditOutlinerView()->GetEditView().SetSelection(ESelection::All()); + pView->GetTextEditOutlinerView()->GetEditView().InsertText(OUString()); + pView->SdrEndTextEdit(); + CPPUNIT_ASSERT(pTitle->IsEmptyPresObj()); + + // Then one undo brings the text back. Without the fix the deletion was not undoable at all: the + // prompt goes in before svx captures the text change, which then looks like no change. + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); + CPPUNIT_ASSERT_EQUAL(u"Typed"_ustr, + pTitle->GetOutlinerParaObject()->GetTextObject().GetText(0)); + CPPUNIT_ASSERT(!pTitle->IsEmptyPresObj()); +} + CPPUNIT_TEST_FIXTURE(SdUiImpressTest, testTdf166401_imageFillsAPlaceholderHoldingText) { // Given a slide whose picture placeholder holds text, so an outliner object represents it: diff --git a/sd/source/core/sdpage.cxx b/sd/source/core/sdpage.cxx index 7142c1a482b5..14e95ebbb97e 100644 --- a/sd/source/core/sdpage.cxx +++ b/sd/source/core/sdpage.cxx @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -2589,6 +2590,19 @@ rtl::Reference SdPage::MakePresObjPlaceholder(SdrObject& rObj) { const PresObjKind eKind = GetPresObjKind(&rObj); + // CreatePresObj records its insert whenever a list action is open, and the object leaves the + // page again below: an insert with no matching remove, which redo would perform a second time. + // The pair cancels out on the page, so the caller's replace is the only undoable step. + SfxUndoManager* pUndoManager + = static_cast(getSdrModelFromSdrPage()).GetUndoManager(); + const bool bUndoWasEnabled = pUndoManager && pUndoManager->IsUndoEnabled(); + if (bUndoWasEnabled) + pUndoManager->EnableUndo(false); + comphelper::ScopeGuard aUndoGuard([pUndoManager, bUndoWasEnabled] { + if (bUndoWasEnabled) + pUndoManager->EnableUndo(true); + }); + // What stands for the placeholder while it waits: for a picture one, the icon and the prompt. SdrObject* pPlaceholder = CreatePresObj(eKind, /*bVertical*/false, rObj.GetLogicRect(), rObj.GetCustomPromptText()); @@ -3197,7 +3211,12 @@ bool SdPage::RestoreDefaultText( SdrObject* pObj, const OUString& rStr ) } pTextObj->SetTextEditOutliner( nullptr ); // to make stylesheet settings work - pTextObj->NbcSetStyleSheet( GetStyleSheetForPresObj(ePresObjKind), true ); + + // A kind with no style sheet of its own keeps the one it has: left with none, the + // object falls back to the pool defaults, which paint a solid fill. + if (SfxStyleSheet* pStyleSheet = GetStyleSheetForPresObj(ePresObjKind)) + pTextObj->NbcSetStyleSheet( pStyleSheet, true ); + pTextObj->SetEmptyPresObj(true); bRet = true; } diff --git a/sd/source/ui/view/sdview.cxx b/sd/source/ui/view/sdview.cxx index 5b02442b2536..b8fca1100228 100644 --- a/sd/source/ui/view/sdview.cxx +++ b/sd/source/ui/view/sdview.cxx @@ -41,6 +41,8 @@ #include #include #include +#include +#include #include #include #include @@ -785,6 +787,22 @@ SdrEndTextEditKind View::SdrEndTextEdit(bool bDontDeleteReally) SdrTextObj* pObj = GetTextEditObject(); + // RestoreDefaultText below puts the prompt back before the text edit ends, which leaves + // SdrObjEditView::SdrEndTextEdit nothing to record, so what the object holds is recorded here. + // The group keeps that in one step with the change of what represents a picture placeholder. + SdPage* pTextPage = pObj ? dynamic_cast(pObj->getSdrPageFromSdrObject()) : nullptr; + const bool bGroupUndo = IsUndoEnabled() && pTextPage + && pTextPage->GetPresObjKind(pObj) != PresObjKind::NONE; + std::unique_ptr pTextUndo; + if (bGroupUndo) + { + BegUndo(SvxResId(STR_UndoObjSetText), pObj->TakeObjNameSingul()); + std::unique_ptr pAction( + GetModel().GetSdrUndoFactory().CreateUndoObjectSetText(*pObj, 0)); + if (dynamic_cast(pAction.get())) + pTextUndo.reset(static_cast(pAction.release())); + } + bool bDefaultTextRestored = RestoreDefaultText( pObj ); const bool bSaveSetModifiedEnabled = mpDocSh && mpDocSh->IsEnableSetModified(); if (bDefaultTextRestored) @@ -820,6 +838,14 @@ SdrEndTextEditKind View::SdrEndTextEdit(bool bDontDeleteReally) } } + // Before the swap below, so that undo restores the object first and its text afterwards. + if (pTextUndo && bDefaultTextRestored) + { + pTextUndo->AfterSetText(); + if (pTextUndo->IsDifferent()) + AddUndo(std::move(pTextUndo)); + } + GetViewShell()->GetViewShellBase().GetEventMultiplexer()->MultiplexEvent( EventMultiplexerEventId::EndTextEdit, static_cast(pObj) ); @@ -861,6 +887,9 @@ SdrEndTextEditKind View::SdrEndTextEdit(bool bDontDeleteReally) } } + if (bGroupUndo) + EndUndo(); + return eKind; }