oox/qa/unit/export.cxx | 21 ++++++++++++- sw/source/filter/ww8/docxattributeoutput.cxx | 28 ++++++++++++++--- sw/source/filter/ww8/wrtw8nds.cxx | 8 +++++ writerfilter/source/dmapper/DomainMapper_Impl.cxx | 35 +++++++++++++++++++++- 4 files changed, 85 insertions(+), 7 deletions(-)
New commits: commit 23a662af7d9385bf20afeab36a684bbdfe48045d Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Sat Feb 17 11:25:37 2024 -0500 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Wed Feb 21 13:34:07 2024 +0100 docx import: correct redline content-controls When inserting and deleting content-controls with change-tracking enabled, we hit a few corner-cases that we need to handle more smartly. First, we shouldn't redline the controls themselves, just the placeholder text. Second, we have to take special care to create valid XML structure with the redline tags. Includes unit-test that reproduces the issues and verifies that both saving and loading work as expected. (cherry picked from commit 1b0f67018fa1d514ebca59e081efdd24c1d7811b) Change-Id: I6af4d0d2c3f0661e7990d5414cc93effc96f0469 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163681 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Miklos Vajna <vmik...@collabora.com> diff --git a/oox/qa/unit/export.cxx b/oox/qa/unit/export.cxx index 4ad6dce4e288..bfee2e7ef8b3 100644 --- a/oox/qa/unit/export.cxx +++ b/oox/qa/unit/export.cxx @@ -9,6 +9,9 @@ #include <test/unoapixml_test.hxx> +#include <com/sun/star/text/ControlCharacter.hpp> +#include <com/sun/star/text/XTextDocument.hpp> + using namespace ::com::sun::star; namespace @@ -97,14 +100,30 @@ CPPUNIT_TEST_FIXTURE(Test, testInsertCheckboxContentControlDocx) { loadFromFile(u"dml-groupshape-polygon.docx"); + uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<text::XText> xText = xTextDocument->getText(); + uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor(); + // With TrackChanges, the Checkbox causes an assertion in the sax serializer, // in void sax_fastparser::FastSaxSerializer::endFastElement(sal_Int32). // Element == maMarkStack.top()->m_DebugStartedElements.back() // sax/source/tools/fastserializer.cxx#402 dispatchCommand(mxComponent, ".uno:TrackChanges", {}); + + xText->insertControlCharacter(xCursor, text::ControlCharacter::PARAGRAPH_BREAK, false); + dispatchCommand(mxComponent, ".uno:InsertCheckboxContentControl", {}); - save("Office Open XML Text"); + // Loading should not show the "corrupted" dialog, which would assert. + saveAndReload("Office Open XML Text"); + + // Now that we loaded it successfully, delete the controls, + // still with change-tracking enabled, and save. + dispatchCommand(mxComponent, ".uno:SelectAll", {}); + dispatchCommand(mxComponent, ".uno:Delete", {}); + + // Loading should not show the "corrupted" dialog, which would assert. + saveAndReload("Office Open XML Text"); // validate(maTempFile.GetFileName(), test::OOXML); // Enable when unrelated errors are fixed. } } diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx index 2bd25dba7b5e..03ada68bcb2d 100644 --- a/sw/source/filter/ww8/docxattributeoutput.cxx +++ b/sw/source/filter/ww8/docxattributeoutput.cxx @@ -1965,7 +1965,22 @@ void DocxAttributeOutput::EndRun(const SwTextNode* pNode, sal_Int32 nPos, sal_In } // if there is some redlining in the document, output it - StartRedline( m_pRedlineData, bLastRun ); + bool bSkipRedline = false; + if (nLen == 1) + { + // Don't redline content-controls--Word doesn't do them. + SwTextAttr* pAttr + = pNode->GetTextAttrAt(nPos, RES_TXTATR_CONTENTCONTROL, sw::GetTextAttrMode::Default); + if (pAttr && pAttr->GetStart() == nPos) + { + bSkipRedline = true; + } + } + + if (!bSkipRedline) + { + StartRedline(m_pRedlineData, bLastRun); + } // XML_r node should be surrounded with bookmark-begin and bookmark-end nodes if it has bookmarks. // The same is applied for permission ranges. @@ -2042,6 +2057,13 @@ void DocxAttributeOutput::EndRun(const SwTextNode* pNode, sal_Int32 nPos, sal_In // append the actual run end m_pSerializer->endElementNS( XML_w, XML_r ); + // if there is some redlining in the document, output it + // (except in the case of fields with multiple runs) + if (!bSkipRedline) + { + EndRedline(m_pRedlineData, bLastRun); + } + if (nLen != -1) { sal_Int32 nEnd = nPos + nLen; @@ -2052,10 +2074,6 @@ void DocxAttributeOutput::EndRun(const SwTextNode* pNode, sal_Int32 nPos, sal_In } } - // if there is some redlining in the document, output it - // (except in the case of fields with multiple runs) - EndRedline( m_pRedlineData, bLastRun ); - // enclose in a sdt block, if necessary: if one is already started, then don't do it for now // (so on export sdt blocks are never nested ATM) if ( !m_bAnchorLinkedToNode && !m_aRunSdt.m_bStartedSdt) diff --git a/sw/source/filter/ww8/wrtw8nds.cxx b/sw/source/filter/ww8/wrtw8nds.cxx index 39e2f885230b..68437c9529b3 100644 --- a/sw/source/filter/ww8/wrtw8nds.cxx +++ b/sw/source/filter/ww8/wrtw8nds.cxx @@ -2478,6 +2478,14 @@ void MSWordExportBase::OutputTextNode( SwTextNode& rNode ) bool bStartedPostponedRunProperties = false; OUString aSavedSnippet ; + // Don't redline content-controls--Word doesn't do them. + SwTextAttr* pAttr = rNode.GetTextAttrAt(nCurrentPos, RES_TXTATR_CONTENTCONTROL, + sw::GetTextAttrMode::Default); + if (pAttr && pAttr->GetStart() == nCurrentPos) + { + pRedlineData = nullptr; + } + sal_Int32 nNextAttr = GetNextPos( &aAttrIter, rNode, nCurrentPos ); // Skip un-exportable attributes. diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index 4ee9f58f3e4c..7a009d71aee8 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -1033,7 +1033,40 @@ void DomainMapper_Impl::PopSdt() uno::Reference<text::XTextRange> xStart = aPosition.m_xTextRange; uno::Reference<text::XTextRange> xEnd = GetTopTextAppend()->getEnd(); uno::Reference<text::XText> xText = xEnd->getText(); - uno::Reference<text::XTextCursor> xCursor = xText->createTextCursorByRange(xStart); + + uno::Reference<text::XTextCursor> xCursor; + try + { + xCursor = xText->createTextCursorByRange(xStart); + } + catch (const uno::RuntimeException&) + { + // We redline form controls and that gets us confused when + // we process the SDT around the placeholder. What seems to + // happen is we lose the text-range when we pop the SDT position. + // Here, we reset the text-range when we fail to create the + // cursor from the top SDT position. + if (m_aTextAppendStack.empty()) + { + return; + } + + uno::Reference<text::XTextAppend> xTextAppend = m_aTextAppendStack.top().xTextAppend; + if (!xTextAppend.is()) + { + return; + } + + uno::Reference<text::XText> xText2 = xTextAppend->getText(); + if (!xText2.is()) + { + return; + } + + // Reset to the start. + xCursor = xText2->createTextCursorByRange(xTextAppend->getStart()); + } + if (!xCursor) { SAL_WARN("writerfilter.dmapper", "DomainMapper_Impl::PopSdt: no start position");