sw/qa/extras/ooxmlexport/data/tdf165059_broken.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport12.cxx | 39 +++-----
sw/qa/extras/ooxmlexport/ooxmlexport21.cxx | 13 ++
sw/source/filter/ww8/docxattributeoutput.cxx | 91 +++++++++++++-------
sw/source/filter/ww8/docxattributeoutput.hxx | 8 +
5 files changed, 94 insertions(+), 57 deletions(-)
New commits:
commit 7b57004cf20ae1715eabd6623ad9007a68a0a32e
Author: Jaume Pujantell <jaume.pujant...@collabora.com>
AuthorDate: Tue Mar 4 23:04:42 2025 +0100
Commit: Jaume Pujantell <jaume.pujant...@collabora.com>
CommitDate: Fri Mar 7 17:31:26 2025 +0100
tdf#165059 sw fix not valid moveFrom/moveTo tag
According to the ISO standard, w:moveFrom/To elements are only permitted
within a move*RangeStart/End section. If not inside such delimited range
only w:ins and w:del are valid.
We still write w:move* in paragraph properties outside of move*Range.
This is technically also not valid, but lots of tests seem to have
documents with this, and it is outside the scope of the bug.
Change-Id: Iaf097b8b017aac1f6182a91abe464433e7664620
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182503
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182633
Reviewed-by: Jaume Pujantell <jaume.pujant...@collabora.com>
Tested-by: Jenkins
diff --git a/sw/qa/extras/ooxmlexport/data/tdf165059_broken.docx
b/sw/qa/extras/ooxmlexport/data/tdf165059_broken.docx
new file mode 100644
index 000000000000..548d4d9c7414
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/tdf165059_broken.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx
index 7eb8f39f4137..e34b17467348 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx
@@ -1229,28 +1229,23 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf145720)
loadAndSave("tdf104797.docx");
xmlDocUniquePtr pXmlDoc = parseExport(u"word/document.xml"_ustr);
// These were 0 (missing move*FromRange* elements)
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart", 1);
+ assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]//w:moveFromRangeStart", 1);
assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:moveFromRangeEnd", 1);
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart", 1);
+ assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]//w:moveToRangeStart", 1);
assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:moveToRangeEnd", 1);
// paired names
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart", "name",
- u"move471382752");
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart", "name",
+ assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:moveFromRangeStart",
"name",
u"move471382752");
+ assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:moveToRangeStart",
"name", u"move471382752");
// mandatory authors and dates
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart", "author",
- u"Tekijä");
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart", "author",
- u"Tekijä");
+ assertXPath(pXmlDoc, "/w:document/w:body/w:p[1]/w:moveFromRangeStart",
"author", u"Tekijä");
+ assertXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:moveToRangeStart",
"author", u"Tekijä");
// no date (anonymized change)
// This failed, date was exported as w:date="0-00-00T00:00:00Z", and later
"1970-01-01T00:00:00Z"
- assertXPathNoAttribute(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart",
- "date");
- assertXPathNoAttribute(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart",
- "date");
+ assertXPathNoAttribute(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFromRangeStart", "date");
+ assertXPathNoAttribute(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveToRangeStart", "date");
}
CPPUNIT_TEST_FIXTURE(Test, testTdf150166)
@@ -1553,11 +1548,10 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf132271)
loadAndSave("tdf149388.docx");
xmlDocUniquePtr pXmlDoc = parseExport(u"word/document.xml"_ustr);
// import change tracking in floating tables
- assertXPath(pXmlDoc, "//w:del", 1);
- assertXPath(pXmlDoc, "//w:ins", 1);
- // tracked text moving recognized during the import
- assertXPath(pXmlDoc, "//w:moveFrom", 1);
- assertXPath(pXmlDoc, "//w:moveTo", 1);
+ assertXPath(pXmlDoc, "//w:del", 2);
+ assertXPath(pXmlDoc, "//w:ins", 2);
+ assertXPath(pXmlDoc, "//w:moveFrom", 0);
+ assertXPath(pXmlDoc, "//w:moveTo", 0);
}
CPPUNIT_TEST_FIXTURE(Test, testTdf149388_fly)
@@ -1578,11 +1572,10 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf136667)
loadAndSave("tdf149388_fly.docx");
xmlDocUniquePtr pXmlDoc = parseExport(u"word/document.xml"_ustr);
// import change tracking in floating tables
- assertXPath(pXmlDoc, "//w:del", 1);
- assertXPath(pXmlDoc, "//w:ins", 3);
- // tracked text moving recognized during the import
- assertXPath(pXmlDoc, "//w:moveFrom", 1);
- assertXPath(pXmlDoc, "//w:moveTo", 1);
+ assertXPath(pXmlDoc, "//w:del", 2);
+ assertXPath(pXmlDoc, "//w:ins", 4);
+ assertXPath(pXmlDoc, "//w:moveFrom", 0);
+ assertXPath(pXmlDoc, "//w:moveTo", 0);
}
CPPUNIT_TEST_FIXTURE(Test, testTdf136850)
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
index d6642b17daa7..d6cf96a8a830 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
@@ -1282,6 +1282,19 @@ CPPUNIT_TEST_FIXTURE(Test, testMsWordUlTrailSpace)
assertXPath(pXmlSettings, "/w:settings/w:compat/w:ulTrailSpace");
}
+CPPUNIT_TEST_FIXTURE(Test, testTdf165059_moveFromTo)
+{
+ loadAndSave("tdf165059_broken.docx");
+ // Without the fix, exported contains w:move* ouside of move ranges
+ // Outside move range tags ins/del are valid
+ xmlDocUniquePtr p_XmlDoc = parseExport("word/document.xml");
+ CPPUNIT_ASSERT(p_XmlDoc);
+ assertXPath(p_XmlDoc, "//w:moveTo"_ostr, 0);
+ assertXPath(p_XmlDoc, "//w:ins"_ostr, 1);
+ assertXPath(p_XmlDoc, "//w:moveFrom"_ostr, 0);
+ assertXPath(p_XmlDoc, "//w:del"_ostr, 1);
+}
+
} // end of anonymous namespace
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx
b/sw/source/filter/ww8/docxattributeoutput.cxx
index 813f1031fd05..6f1aa08eca3f 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -1644,13 +1644,13 @@ void DocxAttributeOutput::EndParagraphProperties(const
SfxItemSet& rParagraphMar
if ( pRedlineParagraphMarkerDeleted )
{
- StartRedline( pRedlineParagraphMarkerDeleted, /*bLastRun=*/true );
- EndRedline( pRedlineParagraphMarkerDeleted, /*bLastRun=*/true );
+ StartRedline(pRedlineParagraphMarkerDeleted, /*bLastRun=*/true,
/*bParagraphProps=*/true);
+ EndRedline(pRedlineParagraphMarkerDeleted, /*bLastRun=*/true,
/*bParagraphProps=*/true);
}
if ( pRedlineParagraphMarkerInserted )
{
- StartRedline( pRedlineParagraphMarkerInserted, /*bLastRun=*/true );
- EndRedline( pRedlineParagraphMarkerInserted, /*bLastRun=*/true );
+ StartRedline(pRedlineParagraphMarkerInserted, /*bLastRun=*/true,
/*bParagraphProps=*/true);
+ EndRedline(pRedlineParagraphMarkerInserted, /*bLastRun=*/true,
/*bParagraphProps=*/true);
}
// mergeTopMarks() after paragraph mark properties child elements.
@@ -1964,6 +1964,13 @@ void DocxAttributeOutput::EndRun(const SwTextNode*
pNode, sal_Int32 nPos, sal_In
m_nHyperLinkCount.back()++;
}
+ // XML_r node should be surrounded with bookmark-begin and bookmark-end
nodes if it has bookmarks.
+ // The same is applied for permission ranges.
+ // But due to unit test "testFdo85542" let's output bookmark-begin with
bookmark-end.
+ DoWriteBookmarksStart(m_rBookmarksStart, m_pMoveRedlineData);
+ DoWriteBookmarksEnd(m_rBookmarksEnd, false, false); // Write non-moverange
bookmarks
+ DoWritePermissionsStart();
+ DoWriteAnnotationMarks();
// if there is some redlining in the document, output it
bool bSkipRedline = false;
if (nLen == 1)
@@ -1982,14 +1989,6 @@ void DocxAttributeOutput::EndRun(const SwTextNode*
pNode, sal_Int32 nPos, sal_In
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.
- // But due to unit test "testFdo85542" let's output bookmark-begin with
bookmark-end.
- DoWriteBookmarksStart(m_rBookmarksStart, m_pMoveRedlineData);
- DoWriteBookmarksEnd(m_rBookmarksEnd);
- DoWritePermissionsStart();
- DoWriteAnnotationMarks();
-
if (m_closeHyperlinkInThisRun && m_nHyperLinkCount.back() > 0 &&
!m_hyperLinkAnchor.isEmpty()
&& m_hyperLinkAnchor.startsWith("_Toc"))
{
@@ -2063,6 +2062,7 @@ void DocxAttributeOutput::EndRun(const SwTextNode* pNode,
sal_Int32 nPos, sal_In
{
EndRedline(m_pRedlineData, bLastRun);
}
+ DoWriteBookmarksEnd(m_rBookmarksEnd, false, true); // Write moverange
bookmarks
if (nLen != -1)
{
@@ -2194,7 +2194,7 @@ void DocxAttributeOutput::EndRun(const SwTextNode* pNode,
sal_Int32 nPos, sal_In
}
DoWriteBookmarksStart(m_rFinalBookmarksStart);
- DoWriteBookmarksEnd(m_rFinalBookmarksEnd);
+ DoWriteBookmarksEnd(m_rFinalBookmarksEnd); // Write all final bookmarks
DoWriteBookmarkEndIfExist(nPos);
}
@@ -2309,28 +2309,37 @@ void
DocxAttributeOutput::DoWriteBookmarksStart(std::vector<OUString>& rStarts,
}
/// export the end bookmarks
-void DocxAttributeOutput::DoWriteBookmarksEnd(std::vector<OUString>& rEnds)
+void DocxAttributeOutput::DoWriteBookmarksEnd(std::vector<OUString>& rEnds,
bool bWriteAllBookmarks,
+ bool bWriteOnlyMoveRanges)
{
- for (const OUString & bookmarkName : rEnds)
+ auto bookmarkNameIt = rEnds.begin();
+ while (bookmarkNameIt != rEnds.end())
{
// Get the id of the bookmark
- auto pPos = m_rOpenedBookmarksIds.find(bookmarkName);
+ auto pPos = m_rOpenedBookmarksIds.find(*bookmarkNameIt);
if (pPos != m_rOpenedBookmarksIds.end())
{
bool bMove = false;
bool bFrom = false;
- GetExport().BookmarkToWord(bookmarkName, &bMove, &bFrom);
+ GetExport().BookmarkToWord(*bookmarkNameIt, &bMove, &bFrom);
// Output the bookmark (including MoveBookmark of the tracked
moving)
- if ( bMove )
- DoWriteMoveRangeTagEnd(pPos->second, bFrom);
- else
- DoWriteBookmarkTagEnd(pPos->second);
+ if (bWriteAllBookmarks || (bMove == bWriteOnlyMoveRanges))
+ {
+ if (bMove)
+ DoWriteMoveRangeTagEnd(pPos->second, bFrom);
+ else
+ DoWriteBookmarkTagEnd(pPos->second);
- m_rOpenedBookmarksIds.erase(bookmarkName);
+ m_rOpenedBookmarksIds.erase(*bookmarkNameIt);
+ bookmarkNameIt = rEnds.erase(bookmarkNameIt);
+ }
+ else
+ ++bookmarkNameIt;
}
+ else
+ bookmarkNameIt = rEnds.erase(bookmarkNameIt);
}
- rEnds.clear();
}
// For construction of the special bookmark name template for permissions:
@@ -4255,7 +4264,8 @@ void DocxAttributeOutput::Redline( const SwRedlineData*
pRedlineData)
// The difference between 'Redline' and 'StartRedline'+'EndRedline' is that:
// 'Redline' is used for tracked changes of formatting information of a run
like Bold, Underline. (the '<w:rPrChange>' is inside the 'run' node)
// 'StartRedline' is used to output tracked changes of run insertion and
deletion (the run is inside the '<w:ins>' node)
-void DocxAttributeOutput::StartRedline( const SwRedlineData * pRedlineData,
bool bLastRun )
+void DocxAttributeOutput::StartRedline(const SwRedlineData* pRedlineData, bool
bLastRun,
+ bool bParagraphProps)
{
if ( !pRedlineData )
return;
@@ -4278,9 +4288,18 @@ void DocxAttributeOutput::StartRedline( const
SwRedlineData * pRedlineData, bool
const DateTime& aDateTime = pRedlineData->GetTimeStamp();
bool bNoDate = bRemovePersonalInfo ||
( aDateTime.GetYear() == 1970 && aDateTime.GetMonth() == 1 &&
aDateTime.GetDay() == 1 );
- bool bMoved = pRedlineData->IsMoved() &&
- // tdf#150166 save tracked moving around TOC as w:ins, w:del
- SwDoc::GetCurTOX(*m_rExport.m_pCurPam->GetPoint()) == nullptr;
+ bool isInMoveBookmark = false;
+ for (const auto& openedBookmark : m_rOpenedBookmarksIds)
+ {
+ if (openedBookmark.first.startsWith(u"__RefMove"))
+ {
+ isInMoveBookmark = true;
+ break;
+ }
+ }
+ bool bMoved = (isInMoveBookmark || bParagraphProps) &&
pRedlineData->IsMoved() &&
+ // tdf#150166 save tracked moving around TOC as w:ins, w:del
+ SwDoc::GetCurTOX(*m_rExport.m_pCurPam->GetPoint()) ==
nullptr;
switch ( pRedlineData->GetType() )
{
case RedlineType::Insert:
@@ -4308,14 +4327,24 @@ void DocxAttributeOutput::StartRedline( const
SwRedlineData * pRedlineData, bool
}
}
-void DocxAttributeOutput::EndRedline( const SwRedlineData * pRedlineData, bool
bLastRun )
+void DocxAttributeOutput::EndRedline(const SwRedlineData* pRedlineData, bool
bLastRun,
+ bool bParagraphProps)
{
if ( !pRedlineData || m_bWritingField )
return;
- bool bMoved = pRedlineData->IsMoved() &&
- // tdf#150166 save tracked moving around TOC as w:ins, w:del
- SwDoc::GetCurTOX(*m_rExport.m_pCurPam->GetPoint()) == nullptr;
+ bool isInMoveBookmark = false;
+ for (const auto& openedBookmark : m_rOpenedBookmarksIds)
+ {
+ if (openedBookmark.first.startsWith(u"__RefMove"))
+ {
+ isInMoveBookmark = true;
+ break;
+ }
+ }
+ bool bMoved = (isInMoveBookmark || bParagraphProps) &&
pRedlineData->IsMoved() &&
+ // tdf#150166 save tracked moving around TOC as w:ins, w:del
+ SwDoc::GetCurTOX(*m_rExport.m_pCurPam->GetPoint()) ==
nullptr;
switch ( pRedlineData->GetType() )
{
case RedlineType::Insert:
diff --git a/sw/source/filter/ww8/docxattributeoutput.hxx
b/sw/source/filter/ww8/docxattributeoutput.hxx
index e19940a673a2..3bd1544003a1 100644
--- a/sw/source/filter/ww8/docxattributeoutput.hxx
+++ b/sw/source/filter/ww8/docxattributeoutput.hxx
@@ -269,12 +269,13 @@ public:
///
/// Start of the tag that encloses the run, fills the info according to
/// the value of pRedlineData.
- void StartRedline( const SwRedlineData * pRedlineData, bool bLastRun );
+ void StartRedline(const SwRedlineData* pRedlineData, bool bLastRun,
+ bool bParagraphProps = false);
/// Output redlining.
///
/// End of the tag that encloses the run.
- void EndRedline( const SwRedlineData * pRedlineData, bool bLastRun );
+ void EndRedline(const SwRedlineData* pRedlineData, bool bLastRun, bool
bParagraphProps = false);
virtual void SetStateOfFlyFrame( FlyProcessingState nStateOfFlyFrame )
override;
virtual void SetAnchorIsLinkedToNode( bool bAnchorLinkedToNode ) override;
@@ -779,7 +780,8 @@ private:
bool bFrom, const SwRedlineData* pRedlineData);
void DoWriteMoveRangeTagEnd(sal_Int32 nId, bool bFrom);
void DoWriteBookmarksStart(std::vector<OUString>& rStarts, const
SwRedlineData* pRedlineData = nullptr);
- void DoWriteBookmarksEnd(std::vector<OUString>& rEnds);
+ void DoWriteBookmarksEnd(std::vector<OUString>& rEnds, bool
bWriteAllBookmarks = true,
+ bool bWriteOnlyMoveRanges = false);
void DoWriteBookmarkStartIfExist(sal_Int32 nRunPos);
void DoWriteBookmarkEndIfExist(sal_Int32 nRunPos);
