tdf#151548 ContentControls: improve Invalidation

This addresses some hack concerns that Miklos raised
in his review.

Change-Id: Ic6d906e9eb1d23700fb743e179ba87899ce2f4a1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143412
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

8 files changed, 45 insertions, 37 deletions

diff --git a/sw/inc/formatcontentcontrol.hxx b/sw/inc/formatcontentcontrol.hxx
index 415132409e10..88d32e83d31a 100644
--- a/sw/inc/formatcontentcontrol.hxx
+++ b/sw/inc/formatcontentcontrol.hxx
@@ -180,9 +180,11 @@ class SW_DLLPUBLIC SwContentControl : public sw::BroadcastingModify
     sal_Int32 m_nId = 0;
 
     /// Stores a list item index, in case the doc model is not yet updated.
+    // i.e. temporarily store the selected item until the text is inserted by GotoContentControl.
     std::optional<size_t> m_oSelectedListItem;
 
     /// Stores a date timestamp, in case the doc model is not yet updated.
+    // i.e. temporarily store the date until the text is inserted by GotoContentControl.
     std::optional<double> m_oSelectedDate;
 
     /**
diff --git a/sw/inc/textcontentcontrol.hxx b/sw/inc/textcontentcontrol.hxx
index 156b44804195..8c8a3b3ee044 100644
--- a/sw/inc/textcontentcontrol.hxx
+++ b/sw/inc/textcontentcontrol.hxx
@@ -42,7 +42,7 @@ public:
     void ChgTextNode(SwTextNode* pNode);
 
     SwTextNode* GetTextNode() const;
-    void Invalidate(bool bKeepPlaceholderStatus = true);
+    void Invalidate();
 
     void dumpAsXml(xmlTextWriterPtr pWriter) const override;
 };
diff --git a/sw/source/core/crsr/crstrvl.cxx b/sw/source/core/crsr/crstrvl.cxx
index 4b07f24a2234..db0ec5b1ae1f 100644
--- a/sw/source/core/crsr/crstrvl.cxx
+++ b/sw/source/core/crsr/crstrvl.cxx
@@ -854,9 +854,6 @@ bool SwCursorShell::GotoFormatContentControl(const SwFormatContentControl& rCont
     sal_Int32 nEnd = *pTextContentControl->End() - 1;
     pCursor->GetMark()->Assign(*pTextNode, nEnd);
 
-    // Assume that once the placeholder is selected, the content is no longer the placeholder.
-    pContentControl->SetShowingPlaceHolder(false);
-
     bool bRet = !pCursor->IsSelOvr();
     if (bRet)
     {
diff --git a/sw/source/core/txtnode/attrcontentcontrol.cxx b/sw/source/core/txtnode/attrcontentcontrol.cxx
index b4e060e75ffc..98cfa13f9611 100644
--- a/sw/source/core/txtnode/attrcontentcontrol.cxx
+++ b/sw/source/core/txtnode/attrcontentcontrol.cxx
@@ -662,33 +662,21 @@ SwTextNode* SwTextContentControl::GetTextNode() const
     return rFormatContentControl.GetTextNode();
 }
 
-void SwTextContentControl::Invalidate(bool bKeepPlaceholderStatus)
+void SwTextContentControl::Invalidate()
 {
     SwDocShell* pDocShell = GetTextNode() ? GetTextNode()->GetDoc().GetDocShell() : nullptr;
     if (!pDocShell || !pDocShell->GetWrtShell())
         return;
 
     // save the cursor
+    // NOTE: needs further testing to see if this is adequate (i.e. in auto-run macros...)
     pDocShell->GetWrtShell()->Push();
 
     // visit the control in the text (which makes any necessary visual changes)
-    // NOTE: simply going to a control indicates cancelling m_bShowingPlaceHolder
+    // NOTE: simply going to a control indicates cancelling ShowingPlaceHolder, unless bOnlyRefresh
+    // NOTE: simply going to a checkbox causes a toggle, unless bOnlyRefresh
     auto& rFormatContentControl = static_cast<SwFormatContentControl&>(GetAttr());
-    std::shared_ptr<SwContentControl> pCC = rFormatContentControl.GetContentControl();
-    bKeepPlaceholderStatus = bKeepPlaceholderStatus && pCC && pCC->GetShowingPlaceHolder();
-
-    // NOTE: simply going to a checkbox causes a toggle
-    const bool bSaveChecked = pCC && pCC->GetChecked();
-    if (pCC && pCC->GetCheckbox())
-        pCC->SetChecked(!bSaveChecked); //do a double-toggle to keep the same value!
-
-    pDocShell->GetWrtShell()->GotoContentControl(rFormatContentControl);
-
-    assert((!pCC || !pCC->GetCheckbox() || bSaveChecked == pCC->GetChecked())
-           && "invalidation implementation changed");
-
-    if (bKeepPlaceholderStatus)
-        pCC->SetShowingPlaceHolder(true);
+    pDocShell->GetWrtShell()->GotoContentControl(rFormatContentControl, /*bOnlyRefresh=*/true);
 
     pDocShell->GetWrtShell()->Pop(SwCursorShell::PopMode::DeleteCurrent);
 }
diff --git a/sw/source/ui/vba/vbacontentcontrol.cxx b/sw/source/ui/vba/vbacontentcontrol.cxx
index 66d104c567e6..d75ce84d7f4a 100644
--- a/sw/source/ui/vba/vbacontentcontrol.cxx
+++ b/sw/source/ui/vba/vbacontentcontrol.cxx
@@ -99,7 +99,8 @@ void SwVbaContentControl::setChecked(sal_Bool bSet)
     if (pCC->GetCheckbox() && pCC->GetChecked() != static_cast<bool>(bSet))
     {
         pCC->SetChecked(bSet);
-        m_rCC.Invalidate(/*bKeepPlaceholderStatus=*/false);
+        pCC->SetShowingPlaceHolder(false);
+        m_rCC.Invalidate();
     }
 }
 
@@ -734,7 +735,7 @@ void SwVbaContentControl::SetPlaceholderText(const uno::Any& BuildingBlock, cons
         // Remove placeholder text.
         pCC->SetPlaceholderDocPart("");
     }
-    m_rCC.Invalidate(/*bKeepPlaceholderStatus=true*/);
+    m_rCC.Invalidate();
 }
 
 void SwVbaContentControl::Ungroup() { SAL_INFO("sw.vba", "SwVbaContentControl::UnGroup stub"); }
diff --git a/sw/source/ui/vba/vbacontentcontrollistentry.cxx b/sw/source/ui/vba/vbacontentcontrollistentry.cxx
index e666bbf0bcd7..61d2a1faf6a8 100644
--- a/sw/source/ui/vba/vbacontentcontrollistentry.cxx
+++ b/sw/source/ui/vba/vbacontentcontrollistentry.cxx
@@ -62,9 +62,20 @@ void SwVbaContentControlListEntry::setText(const OUString& rSet)
         if (vListItems[i].ToString() == rSet)
             return;
     }
+
+    bool bNeedsInvalidation = false;
+    if (!pCC->GetShowingPlaceHolder())
+    {
+        // TODO: implement bCheckDocModel
+        std::optional<size_t> oSel(pCC->GetSelectedListItem(/*bCheckDocModel=true*/));
+        bNeedsInvalidation = oSel && *oSel == m_nZIndex;
+    }
+
     vListItems[m_nZIndex].m_aDisplayText = rSet;
     pCC->SetListItems(vListItems);
-    m_rCC.Invalidate();
+
+    if (bNeedsInvalidation)
+        m_rCC.Invalidate();
 }
 
 OUString SwVbaContentControlListEntry::getValue()
@@ -143,7 +154,8 @@ void SwVbaContentControlListEntry::Select()
     std::shared_ptr<SwContentControl> pCC = m_rCC.GetContentControl().GetContentControl();
     assert(m_nZIndex < pCC->GetListItems().size());
     pCC->SetSelectedListItem(m_nZIndex);
-    m_rCC.Invalidate(/*bKeepPlaceholderStatus=*/false);
+    pCC->SetShowingPlaceHolder(false);
+    m_rCC.Invalidate();
 }
 
 // XHelperInterface
diff --git a/sw/source/uibase/inc/wrtsh.hxx b/sw/source/uibase/inc/wrtsh.hxx
index fb80f135237e..98b3d9038204 100644
--- a/sw/source/uibase/inc/wrtsh.hxx
+++ b/sw/source/uibase/inc/wrtsh.hxx
@@ -428,7 +428,12 @@ typedef bool (SwWrtShell::*FNSimpleMove)();
 
     bool GotoField( const SwFormatField& rField );
 
-    bool GotoContentControl(const SwFormatContentControl& rContentControl);
+    /** @param bOnlyRefresh:
+     *         false: run default actions (e.g. toggle checkbox, remove placeholder content)
+     *         true: do not alter the content control, just refresh the doc model
+     */
+    bool GotoContentControl(const SwFormatContentControl& rContentControl,
+                            bool bOnlyRefresh = false);
 
     // jump to the next / previous hyperlink - inside text and also
     // on graphics
diff --git a/sw/source/uibase/wrtsh/wrtsh3.cxx b/sw/source/uibase/wrtsh/wrtsh3.cxx
index 58204886933e..aa170d1374a4 100644
--- a/sw/source/uibase/wrtsh/wrtsh3.cxx
+++ b/sw/source/uibase/wrtsh/wrtsh3.cxx
@@ -93,7 +93,8 @@ bool SwWrtShell::GotoField( const SwFormatField& rField )
     return bRet;
 }
 
-bool SwWrtShell::GotoContentControl(const SwFormatContentControl& rContentControl)
+bool SwWrtShell::GotoContentControl(const SwFormatContentControl& rContentControl,
+                                    bool bOnlyRefresh)
 {
     std::shared_ptr<SwContentControl> pContentControl = rContentControl.GetContentControl();
     if (IsFrameSelected() && pContentControl && pContentControl->GetPicture())
@@ -124,23 +125,24 @@ bool SwWrtShell::GotoContentControl(const SwFormatContentControl& rContentContro
     (this->*m_fnKillSel)(nullptr, false);
 
     bool bRet = SwCursorShell::GotoFormatContentControl(rContentControl);
+    // Assume that once the placeholder is selected, the content is no longer the placeholder.
+    if (!bOnlyRefresh && pContentControl)
+        pContentControl->SetShowingPlaceHolder(false);
+
 
     if (bRet && pContentControl && pContentControl->GetCheckbox())
     {
         // Checkbox: GotoFormatContentControl() selected the old state.
         LockView(/*bViewLocked=*/true);
-        OUString aOldState;
+        OUString aOldState = GetCursorDescr();
         OUString aNewState;
         if (pContentControl->GetChecked())
-        {
-            aOldState = pContentControl->GetCheckedState();
-            aNewState = pContentControl->GetUncheckedState();
-        }
+            aNewState = bOnlyRefresh ? pContentControl->GetCheckedState()
+                                     : pContentControl->GetUncheckedState();
         else
-        {
-            aOldState = pContentControl->GetUncheckedState();
-            aNewState = pContentControl->GetCheckedState();
-        }
+            aNewState = bOnlyRefresh ? pContentControl->GetUncheckedState()
+                                     : pContentControl->GetCheckedState();
+
         SwRewriter aRewriter;
         aRewriter.AddRule(UndoArg1, aOldState);
         aRewriter.AddRule(UndoArg2, SwResId(STR_YIELDS));
@@ -150,7 +152,8 @@ bool SwWrtShell::GotoContentControl(const SwFormatContentControl& rContentContro
         // Toggle the state.
         pContentControl->SetReadWrite(true);
         DelLeft();
-        pContentControl->SetChecked(!pContentControl->GetChecked());
+        if (!bOnlyRefresh)
+            pContentControl->SetChecked(!pContentControl->GetChecked());
         Insert(aNewState);
         pContentControl->SetReadWrite(false);