sw content controls: fixes for the ending dummy char

- make sure the DOC/RTF export doesn't write the dummy char as-is

- let "enter" only insert a linebreak while inside a content control, to
ensure that the starting and ending dummy char stays inside the same
text node

- let deletion of the dummy character at the end behave the same as the
start dummy character: if trying to delete that single character, then
just move the cursor, don't delete it

- reject document insertion in the middle of a content control, similar
to input fields

(cherry picked from commit 32dab3228cd315437efe0c5b850d116235eaa797)

Conflicts:
sw/qa/core/unocore/unocore.cxx
sw/source/core/unocore/unocrsrhelper.cxx

Change-Id: I9b54ef50261e6b17f38eadadacfe1e1111199e96
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134261
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

15 files changed, 222 insertions, 3 deletions

diff --git a/sw/inc/crsrsh.hxx b/sw/inc/crsrsh.hxx
index a34e02d45ead..2dd27529810a 100644
--- a/sw/inc/crsrsh.hxx
+++ b/sw/inc/crsrsh.hxx
@@ -717,6 +717,7 @@ public:
         const bool bIncludeInputFieldAtStart );
     SwField* GetCurField( const bool bIncludeInputFieldAtStart = false ) const;
     bool CursorInsideInputField() const;
+    bool CursorInsideContentControl() const;
     static bool PosInsideInputField( const SwPosition& rPos );
     bool DocPtInsideInputField( const Point& rDocPt ) const;
     static sal_Int32 StartOfInputFieldAtPos( const SwPosition& rPos );
diff --git a/sw/inc/ndtxt.hxx b/sw/inc/ndtxt.hxx
index c77e062d80cd..9e83ded061f6 100644
--- a/sw/inc/ndtxt.hxx
+++ b/sw/inc/ndtxt.hxx
@@ -403,6 +403,14 @@ public:
         const sal_Int32 nIndex,
         const sal_uInt16 nWhich = RES_TXTATR_END ) const;
 
+    /**
+     * Get the text attribute of an end dummy character at nIndex. Return the attribute only in
+     * case its which id is nWhich.
+     *
+     * Note that the position of the end dummy character is one less than the end of the attribute.
+     */
+    SwTextAttr* GetTextAttrForEndCharAt(sal_Int32 nIndex, sal_uInt16 nWhich) const;
+
     SwTextField* GetFieldTextAttrAt(
         const sal_Int32 nIndex,
         const bool bIncludeInputFieldAtStart = false ) const;
diff --git a/sw/qa/core/crsr/crsr.cxx b/sw/qa/core/crsr/crsr.cxx
index 0b93fcc43522..62cafc6d63a5 100644
--- a/sw/qa/core/crsr/crsr.cxx
+++ b/sw/qa/core/crsr/crsr.cxx
@@ -23,6 +23,7 @@
 #include <docsh.hxx>
 #include <unotxdoc.hxx>
 #include <wrtsh.hxx>
+#include <ndtxt.hxx>
 
 constexpr OUStringLiteral DATA_DIRECTORY = u"/sw/qa/core/crsr/data/";
 
@@ -104,6 +105,37 @@ CPPUNIT_TEST_FIXTURE(SwCoreCrsrTest, testSelAllStartsWithTable)
     CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), pDoc->GetTableFrameFormatCount(/*bUsed=*/true));
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreCrsrTest, testContentControlLineBreak)
+{
+    // Given a document with a (rich text) content control:
+    SwDoc* pDoc = createSwDoc();
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    xText->insertString(xCursor, "test", /*bAbsorb=*/false);
+    xCursor->gotoStart(/*bExpand=*/false);
+    xCursor->gotoEnd(/*bExpand=*/true);
+    uno::Reference<text::XTextContent> xContentControl(
+        xMSF->createInstance("com.sun.star.text.ContentControl"), uno::UNO_QUERY);
+    xText->insertTextContent(xCursor, xContentControl, /*bAbsorb=*/true);
+
+    // When pressing "enter" in the middle of that content control:
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    // Go after "t".
+    pWrtShell->Right(CRSR_SKIP_CHARS, /*bSelect=*/false, 2, /*bBasicCall=*/false);
+    dispatchCommand(mxComponent, ".uno:InsertPara", {});
+
+    // Then make sure that we only insert a line break, not a new paragraph:
+    SwTextNode* pTextNode = pWrtShell->GetCursor()->GetMark()->nNode.GetNode().GetTextNode();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: t\nest
+    // - Actual  : est
+    // i.e. a new paragraph was inserted, which is not allowed for inline content controls.
+    CPPUNIT_ASSERT_EQUAL(OUString("t\nest"), pTextNode->GetExpandText(pWrtShell->GetLayout()));
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx
index 43894afa03dc..26d02e60318b 100644
--- a/sw/qa/core/doc/doc.cxx
+++ b/sw/qa/core/doc/doc.cxx
@@ -225,6 +225,36 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testImageHyperlinkStyle)
     CPPUNIT_ASSERT_EQUAL(aExpected, aActual);
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testContentControlDelete)
+{
+    // Given a document with a content control:
+    SwDoc* pDoc = createSwDoc();
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    xText->insertString(xCursor, "test", /*bAbsorb=*/false);
+    xCursor->gotoStart(/*bExpand=*/false);
+    xCursor->gotoEnd(/*bExpand=*/true);
+    uno::Reference<text::XTextContent> xContentControl(
+        xMSF->createInstance("com.sun.star.text.ContentControl"), uno::UNO_QUERY);
+    xText->insertTextContent(xCursor, xContentControl, /*bAbsorb=*/true);
+
+    // When deleting the dummy character at the end of the content control:
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    pWrtShell->DelLeft();
+
+    // Then make sure that we only enter the content control, to be consistent with the start dummy
+    // character:
+    SwTextNode* pTextNode = pWrtShell->GetCursor()->GetMark()->nNode.GetNode().GetTextNode();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: ^Atest^A
+    // - Actual  : ^Atest
+    // i.e. the end dummy character got deleted, but not the first one, which is inconsistent.
+    CPPUNIT_ASSERT_EQUAL(OUString("\x0001test\x0001"), pTextNode->GetText());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/qa/core/unocore/unocore.cxx b/sw/qa/core/unocore/unocore.cxx
index bfeaea521c6e..8eb74623ad28 100644
--- a/sw/qa/core/unocore/unocore.cxx
+++ b/sw/qa/core/unocore/unocore.cxx
@@ -13,6 +13,7 @@
 #include <com/sun/star/text/XTextAppend.hpp>
 #include <com/sun/star/text/XTextFrame.hpp>
 #include <com/sun/star/text/XTextViewCursorSupplier.hpp>
+#include <com/sun/star/document/XDocumentInsertable.hpp>
 
 #include <comphelper/propertyvalue.hxx>
 #include <comphelper/sequenceashashmap.hxx>
@@ -464,6 +465,32 @@ CPPUNIT_TEST_FIXTURE(SwCoreUnocoreTest, testContentControlDropdown)
     CPPUNIT_ASSERT_EQUAL(OUString("R"), aListItems[0].m_aValue);
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreUnocoreTest, testInsertFileInContentControlException)
+{
+    // Given a document with a content control:
+    createSwDoc();
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    xText->insertString(xCursor, "test", /*bAbsorb=*/false);
+    xCursor->gotoStart(/*bExpand=*/false);
+    xCursor->gotoEnd(/*bExpand=*/true);
+    uno::Reference<text::XTextContent> xContentControl(
+        xMSF->createInstance("com.sun.star.text.ContentControl"), uno::UNO_QUERY);
+    xText->insertTextContent(xCursor, xContentControl, /*bAbsorb=*/true);
+
+    // Reject inserting a document inside the content control:
+    xCursor->goLeft(1, false);
+    OUString aURL(m_directories.getURLFromSrc(DATA_DIRECTORY) + "tdf119081.odt");
+    uno::Reference<document::XDocumentInsertable> xInsertable(xCursor, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_THROW(xInsertable->insertDocumentFromURL(aURL, {}), uno::RuntimeException);
+
+    // Accept inserting a document outside the content control:
+    xCursor->goRight(1, false);
+    xInsertable->insertDocumentFromURL(aURL, {});
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/qa/extras/ww8export/ww8export2.cxx b/sw/qa/extras/ww8export/ww8export2.cxx
index 723fbf43a4c3..4fb6349eba11 100644
--- a/sw/qa/extras/ww8export/ww8export2.cxx
+++ b/sw/qa/extras/ww8export/ww8export2.cxx
@@ -1070,6 +1070,34 @@ DECLARE_WW8EXPORT_TEST(testTdf118412, "tdf118412.doc")
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1251), nBottomMargin);
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testContentControlExport)
+{
+    // Given a document with a (rich text) content control:
+    mxComponent = loadFromDesktop("private:factory/swriter");
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    xText->insertString(xCursor, "test", /*bAbsorb=*/false);
+    xCursor->gotoStart(/*bExpand=*/false);
+    xCursor->gotoEnd(/*bExpand=*/true);
+    uno::Reference<text::XTextContent> xContentControl(
+        xMSF->createInstance("com.sun.star.text.ContentControl"), uno::UNO_QUERY);
+    xText->insertTextContent(xCursor, xContentControl, /*bAbsorb=*/true);
+
+    // When saving that document to DOC and loading it back:
+    reload("MS Word 97", "");
+
+    // Then make sure the dummy character at the end is filtered out:
+    OUString aBodyText = getBodyText();
+    // Without the accompanying fix in place, this test would have failed:
+    // - Expected: test
+    // - Actual  : test<space>
+    // i.e. the CH_TXTATR_BREAKWORD at the end was written, then the import replaced that with a
+    // space.
+    CPPUNIT_ASSERT_EQUAL(OUString("test"), aBodyText);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/qa/inc/swmodeltestbase.hxx b/sw/qa/inc/swmodeltestbase.hxx
index 7a9ac5dcc8a2..f1e8444c4c52 100644
--- a/sw/qa/inc/swmodeltestbase.hxx
+++ b/sw/qa/inc/swmodeltestbase.hxx
@@ -235,6 +235,8 @@ protected:
     /// Get the length of the whole document.
     int getLength() const;
 
+    OUString getBodyText() const;
+
     /// Get a family of styles, see com.sun.star.style.StyleFamilies for possible values.
     css::uno::Reference<css::container::XNameAccess> getStyles(const OUString& aFamily);
 
diff --git a/sw/qa/unit/swmodeltestbase.cxx b/sw/qa/unit/swmodeltestbase.cxx
index 8a8a7d2df5a2..fdf25ca3bc11 100644
--- a/sw/qa/unit/swmodeltestbase.cxx
+++ b/sw/qa/unit/swmodeltestbase.cxx
@@ -193,6 +193,27 @@ int SwModelTestBase::getLength() const
     return aBuf.getLength();
 }
 
+OUString SwModelTestBase::getBodyText() const
+{
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xTextDocument->getText(),
+                                                                  uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration();
+    OUStringBuffer aBuf;
+    while (xParaEnum->hasMoreElements())
+    {
+        uno::Reference<container::XEnumerationAccess> xRangeEnumAccess(xParaEnum->nextElement(),
+                                                                       uno::UNO_QUERY);
+        uno::Reference<container::XEnumeration> xRangeEnum = xRangeEnumAccess->createEnumeration();
+        while (xRangeEnum->hasMoreElements())
+        {
+            uno::Reference<text::XTextRange> xRange(xRangeEnum->nextElement(), uno::UNO_QUERY);
+            aBuf.append(xRange->getString());
+        }
+    }
+    return aBuf.makeStringAndClear();
+}
+
 uno::Reference<container::XNameAccess> SwModelTestBase::getStyles(const OUString& aFamily)
 {
     uno::Reference<style::XStyleFamiliesSupplier> xStyleFamiliesSupplier(mxComponent,
diff --git a/sw/source/core/crsr/crstrvl.cxx b/sw/source/core/crsr/crstrvl.cxx
index 4efaab1637a4..4c3f8eb16fb2 100644
--- a/sw/source/core/crsr/crstrvl.cxx
+++ b/sw/source/core/crsr/crstrvl.cxx
@@ -1002,6 +1002,27 @@ bool SwCursorShell::CursorInsideInputField() const
     return false;
 }
 
+bool SwCursorShell::CursorInsideContentControl() const
+{
+    for (SwPaM& rCursor : GetCursor()->GetRingContainer())
+    {
+        const SwPosition* pStart = rCursor.Start();
+        SwTextNode* pTextNode = pStart->nNode.GetNode().GetTextNode();
+        if (!pTextNode)
+        {
+            continue;
+        }
+
+        sal_Int32 nIndex = pStart->nContent.GetIndex();
+        if (pTextNode->GetTextAttrAt(nIndex, RES_TXTATR_CONTENTCONTROL, SwTextNode::PARENT))
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 bool SwCursorShell::PosInsideInputField( const SwPosition& rPos )
 {
     return dynamic_cast<const SwTextInputField*>(GetTextFieldAtPos( &rPos, false )) != nullptr;
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index b3f76c03a4c3..286381f56158 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -555,12 +555,22 @@ namespace sw
                             // at the end, so no need to check in nStartNode
                             if (n == nEndNode && !isOnlyFieldmarks)
                             {
-                                SwTextAttr const*const pAttr(rTextNode.GetTextAttrForCharAt(i));
+                                SwTextAttr const* pAttr(rTextNode.GetTextAttrForCharAt(i));
                                 if (pAttr && pAttr->End() && (nEnd  < *pAttr->End()))
                                 {
                                     assert(pAttr->HasDummyChar());
                                     rBreaks.emplace_back(n, i);
                                 }
+
+                                if (!pAttr)
+                                {
+                                    // See if this is an end dummy character for a content control.
+                                    pAttr = rTextNode.GetTextAttrForEndCharAt(i, RES_TXTATR_CONTENTCONTROL);
+                                    if (pAttr && (nStart > pAttr->GetStart()))
+                                    {
+                                        rBreaks.emplace_back(n, i);
+                                    }
+                                }
                             }
                             break;
                         }
diff --git a/sw/source/core/txtnode/ndtxt.cxx b/sw/source/core/txtnode/ndtxt.cxx
index 97d65e48fbf0..45b51cfdd744 100644
--- a/sw/source/core/txtnode/ndtxt.cxx
+++ b/sw/source/core/txtnode/ndtxt.cxx
@@ -3089,6 +3089,28 @@ SwTextAttr * SwTextNode::GetTextAttrForCharAt(
     return nullptr;
 }
 
+SwTextAttr* SwTextNode::GetTextAttrForEndCharAt(sal_Int32 nIndex, sal_uInt16 nWhich) const
+{
+    SwTextAttr* pAttr = GetTextAttrAt(nIndex, nWhich, SwTextNode::EXPAND);
+    if (!pAttr)
+    {
+        return nullptr;
+    }
+
+    if (!pAttr->End())
+    {
+        return nullptr;
+    }
+
+    // The start-end range covers the end dummy character.
+    if (*pAttr->End() - 1 != nIndex)
+    {
+        return nullptr;
+    }
+
+    return pAttr;
+}
+
 namespace
 {
 
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index 9f6884d7d087..e381b7a00d4d 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -1026,6 +1026,15 @@ void resetCursorPropertyValue(const SfxItemPropertyMapEntry& rEntry, SwPaM& rPam
 void InsertFile(SwUnoCursor* pUnoCursor, const OUString& rURL,
     const uno::Sequence< beans::PropertyValue >& rOptions)
 {
+    if (SwTextNode const*const pTextNode = pUnoCursor->GetPoint()->nNode.GetNode().GetTextNode())
+    {
+        if (pTextNode->GetTextAttrAt(pUnoCursor->GetPoint()->nContent.GetIndex(),
+                                     RES_TXTATR_CONTENTCONTROL, SwTextNode::PARENT))
+        {
+            throw uno::RuntimeException("cannot insert file inside content controls");
+        }
+    }
+
     std::unique_ptr<SfxMedium> pMed;
     SwDoc& rDoc = pUnoCursor->GetDoc();
     SwDocShell* pDocSh = rDoc.GetDocShell();
diff --git a/sw/source/filter/ww8/wrtw8nds.cxx b/sw/source/filter/ww8/wrtw8nds.cxx
index 62a220a70f08..7695942265b0 100644
--- a/sw/source/filter/ww8/wrtw8nds.cxx
+++ b/sw/source/filter/ww8/wrtw8nds.cxx
@@ -85,6 +85,7 @@
 #include <oox/export/vmlexport.hxx>
 #include <sal/log.hxx>
 #include <comphelper/propertysequence.hxx>
+#include <comphelper/string.hxx>
 
 #include "sprmids.hxx"
 
@@ -1799,6 +1800,12 @@ OUString SwWW8AttrIter::GetSnippet(const OUString &rStr, sal_Int32 nCurrentPos,
     aSnippet = aSnippet.replace(0x0A, 0x0B);
     aSnippet = aSnippet.replace(CHAR_HARDHYPHEN, 0x1e);
     aSnippet = aSnippet.replace(CHAR_SOFTHYPHEN, 0x1f);
+    // Ignore the dummy character at the end of content controls.
+    static sal_Unicode const aForbidden[] = {
+        CH_TXTATR_BREAKWORD,
+        0
+    };
+    aSnippet = comphelper::string::removeAny(aSnippet, aForbidden);
 
     m_rExport.m_aCurrentCharPropStarts.push( nCurrentPos );
     const SfxPoolItem &rItem = GetItem(RES_CHRATR_CASEMAP);
diff --git a/sw/source/uibase/docvw/edtwin.cxx b/sw/source/uibase/docvw/edtwin.cxx
index e663e376b2f4..0f077e3657c0 100644
--- a/sw/source/uibase/docvw/edtwin.cxx
+++ b/sw/source/uibase/docvw/edtwin.cxx
@@ -1856,7 +1856,8 @@ KEYINPUT_CHECKTABLE_INSDEL:
                 case KEY_RETURN:
                 {
                     if ( !rSh.HasReadonlySel()
-                         && !rSh.CursorInsideInputField() )
+                         && !rSh.CursorInsideInputField()
+                         && !rSh.CursorInsideContentControl() )
                     {
                         const SelectionType nSelectionType = rSh.GetSelectionType();
                         if(nSelectionType & SelectionType::Ole)
diff --git a/sw/source/uibase/shells/textsh.cxx b/sw/source/uibase/shells/textsh.cxx
index b7e25a137cef..168f551d8bd1 100644
--- a/sw/source/uibase/shells/textsh.cxx
+++ b/sw/source/uibase/shells/textsh.cxx
@@ -197,7 +197,7 @@ void SwTextShell::ExecInsert(SfxRequest &rReq)
 
     case FN_INSERT_BREAK:
         {
-            if( !rSh.CursorInsideInputField() )
+            if (!rSh.CursorInsideInputField() && !rSh.CursorInsideContentControl())
             {
                 rSh.SplitNode();
             }