sw/qa/extras/ooxmlexport/data/tdf165059_broken.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport12.cxx | 66 ++++----------
sw/qa/extras/ooxmlexport/ooxmlexport21.cxx | 15 +++
sw/source/filter/ww8/docxattributeoutput.cxx | 91 +++++++++++++-------
sw/source/filter/ww8/docxattributeoutput.hxx | 8 +
5 files changed, 102 insertions(+), 78 deletions(-)
New commits:
commit 2189df38ae4aa6613b5ddc7cba93fb780adb35eb
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 09:01:44 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-by: Miklos Vajna <vmik...@collabora.com>
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 aff6a689a9dd..9a32067ce0d6 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport12.cxx
@@ -1224,28 +1224,28 @@ DECLARE_OOXMLEXPORT_TEST(testTdf145720,
"tdf104797.docx")
{
xmlDocUniquePtr pXmlDoc = parseExport("word/document.xml");
// These were 0 (missing move*FromRange* elements)
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart"_ostr, 1);
+ assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFromRangeStart"_ostr, 1);
assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFromRangeEnd"_ostr, 1);
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart"_ostr, 1);
+ assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveToRangeStart"_ostr, 1);
assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveToRangeEnd"_ostr, 1);
// paired names
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart"_ostr,
- "name"_ostr, "move471382752");
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart"_ostr,
- "name"_ostr, "move471382752");
+ assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFromRangeStart"_ostr, "name"_ostr,
+ "move471382752");
+ assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveToRangeStart"_ostr, "name"_ostr,
+ "move471382752");
// mandatory authors and dates
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFrom/w:moveFromRangeStart"_ostr,
- "author"_ostr, u"Tekijä"_ustr);
- assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart"_ostr,
- "author"_ostr, u"Tekijä"_ustr);
+ assertXPath(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFromRangeStart"_ostr, "author"_ostr,
+ u"Tekijä"_ustr);
+ assertXPath(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveToRangeStart"_ostr, "author"_ostr,
+ u"Tekijä"_ustr);
// 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"_ostr, "date"_ostr);
- assertXPathNoAttribute(
- pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveTo/w:moveToRangeStart"_ostr, "date"_ostr);
+ assertXPathNoAttribute(pXmlDoc,
"/w:document/w:body/w:p[1]/w:moveFromRangeStart"_ostr,
+ "date"_ostr);
+ assertXPathNoAttribute(pXmlDoc,
"/w:document/w:body/w:p[2]/w:moveToRangeStart"_ostr,
+ "date"_ostr);
}
}
@@ -1562,21 +1562,10 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf132271)
loadAndSave("tdf149388.docx");
xmlDocUniquePtr pXmlDoc = parseExport("word/document.xml");
// import change tracking in floating tables
- if (!isExported())
- {
- assertXPath(pXmlDoc, "//w:del"_ostr, 2);
- assertXPath(pXmlDoc, "//w:ins"_ostr, 2);
- assertXPath(pXmlDoc, "//w:moveFrom"_ostr, 0);
- assertXPath(pXmlDoc, "//w:moveTo"_ostr, 0);
- }
- else
- {
- assertXPath(pXmlDoc, "//w:del"_ostr, 1);
- assertXPath(pXmlDoc, "//w:ins"_ostr, 1);
- // tracked text moving recognized during the import
- assertXPath(pXmlDoc, "//w:moveFrom"_ostr, 1);
- assertXPath(pXmlDoc, "//w:moveTo"_ostr, 1);
- }
+ assertXPath(pXmlDoc, "//w:del"_ostr, 2);
+ assertXPath(pXmlDoc, "//w:ins"_ostr, 2);
+ assertXPath(pXmlDoc, "//w:moveFrom"_ostr, 0);
+ assertXPath(pXmlDoc, "//w:moveTo"_ostr, 0);
}
CPPUNIT_TEST_FIXTURE(Test, testTdf149388_fly)
@@ -1597,21 +1586,10 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf136667)
loadAndSave("tdf149388_fly.docx");
xmlDocUniquePtr pXmlDoc = parseExport("word/document.xml");
// import change tracking in floating tables
- if (!isExported())
- {
- assertXPath(pXmlDoc, "//w:del"_ostr, 2);
- assertXPath(pXmlDoc, "//w:ins"_ostr, 4);
- assertXPath(pXmlDoc, "//w:moveFrom"_ostr, 0);
- assertXPath(pXmlDoc, "//w:moveTo"_ostr, 0);
- }
- else
- {
- assertXPath(pXmlDoc, "//w:del"_ostr, 1);
- assertXPath(pXmlDoc, "//w:ins"_ostr, 3);
- // tracked text moving recognized during the import
- assertXPath(pXmlDoc, "//w:moveFrom"_ostr, 1);
- assertXPath(pXmlDoc, "//w:moveTo"_ostr, 1);
- }
+ assertXPath(pXmlDoc, "//w:del"_ostr, 2);
+ assertXPath(pXmlDoc, "//w:ins"_ostr, 4);
+ assertXPath(pXmlDoc, "//w:moveFrom"_ostr, 0);
+ assertXPath(pXmlDoc, "//w:moveTo"_ostr, 0);
}
CPPUNIT_TEST_FIXTURE(Test, testTdf136850)
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
index 4c697df716bc..a97720dc69d4 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
@@ -494,6 +494,21 @@ CPPUNIT_TEST_FIXTURE(Test, testMsWordUlTrailSpace)
assertXPath(pXmlSettings, "/w:settings/w:compat/w:ulTrailSpace"_ostr);
}
+DECLARE_OOXMLEXPORT_TEST(testTdf165059_moveFromTo, "tdf165059_broken.docx")
+{
+ if (isExported())
+ {
+ // 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 2e4b2367c064..ed2273925059 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -1653,13 +1653,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.
@@ -1971,6 +1971,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)
@@ -1989,14 +1996,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"))
{
@@ -2070,6 +2069,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)
{
@@ -2201,7 +2201,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);
}
@@ -2316,28 +2316,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:
@@ -4199,7 +4208,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;
@@ -4222,9 +4232,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:
@@ -4252,14 +4271,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 2d9962d18e50..59f78da78437 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;
@@ -780,7 +781,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);
