tdf#145718 sw, DOCX import: complete tracked text moving

Add IsMoved bit to SwRangeRedline, and keep it in both
parts of a split Delete/Insert redline. Set this bit
during DOCX import, fixing incomplete import of
moveFrom/moveTo elements.

Details:

- Search text moving only at redline Insert() and AppendRedline()
  instead in the layout code (which was much slower, because
  triggered by also mouse hovering):

- detect text moving in Hide Changes mode, too;

- Insertion inside or directly after tracked text moving keeps
  "moved text" layout of the original moved text parts (before
  and after the insertion).

- at detection of text moving, invalidate (update) layout of the
  redline pair, too.

- fix DOCX import: extend makeRedline() with property RedlineMoved
  to keep all moveFrom/moveTo stored in DOCX instead of
  losing them (joining them with normal redlines) in the case
  of missing Delete/Insert pair (see unit test document);

Follow-up to commit ec577f566fa3e6d2666069180f8ec8474054aea9
"tdf#145233 sw track changes: show moved text in green color",
commit bcdebc832b272662d28035007a4796e42d1305ae
"tdf#104797 DOCX change tracking: handle moveFrom and moveTo"
and commit d32d9a2b3c5e3963f4a18f6c7bbf50fab2e9b2be
"tdf#123460 DOCX track changes: moveFrom completely".

Change-Id: Iaca80e5e326a172bc7ba5fec64b63668b9378e2d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/125317
Tested-by: László Németh <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>

13 files changed, 103 insertions, 17 deletions

diff --git a/offapi/com/sun/star/text/XRedline.idl b/offapi/com/sun/star/text/XRedline.idl
index 9b259e8b9e58..a07c92ddfc62 100644
--- a/offapi/com/sun/star/text/XRedline.idl
+++ b/offapi/com/sun/star/text/XRedline.idl
@@ -46,6 +46,7 @@ published interface XRedline
                 [readonly, property] string RedlineAuthor;
                 [readonly, property] com::sun::star::util::DateTime RedlineDateTime;
                 [readonly, property] string RedlineComment;
+                [readonly, optional, property] boolean RedlineMoved;
         */
         void makeRedline( [in]string RedlineType, [in] com::sun::star::beans::PropertyValues  RedlineProperties)
                         raises( com::sun::star::lang::IllegalArgumentException );
diff --git a/sw/inc/redline.hxx b/sw/inc/redline.hxx
index 0c5b8408d54c..8d17948205fa 100644
--- a/sw/inc/redline.hxx
+++ b/sw/inc/redline.hxx
@@ -155,6 +155,7 @@ class SW_DLLPUBLIC SwRangeRedline final : public SwPaM
     SwNodeIndex* m_pContentSect;
     bool m_bDelLastPara : 1;
     bool m_bIsVisible : 1;
+    bool m_bIsMoved : 1;
     sal_uInt32 m_nId;
 
     std::optional<tools::Long> m_oLOKLastNodeTop;
@@ -174,7 +175,7 @@ public:
     SwRangeRedline(SwRedlineData* pData, const SwPosition& rPos,
                bool bDelLP) :
         SwPaM( rPos ), m_pRedlineData( pData ), m_pContentSect( nullptr ),
-        m_bDelLastPara( bDelLP ), m_bIsVisible( true ), m_nId( s_nLastId++ )
+        m_bDelLastPara( bDelLP ), m_bIsVisible( true ), m_bIsMoved( false ), m_nId( s_nLastId++ )
     {}
     SwRangeRedline( const SwRangeRedline& );
     virtual ~SwRangeRedline() override;
@@ -261,6 +262,9 @@ public:
     void dumpAsXml(xmlTextWriterPtr pWriter) const;
 
     void MaybeNotifyRedlinePositionModification(tools::Long nTop);
+
+    void SetMoved() { m_bIsMoved = true; }
+    bool IsMoved() const { return m_bIsMoved; }
 };
 
 void MaybeNotifyRedlineModification(SwRangeRedline& rRedline, SwDoc& rDoc);
diff --git a/sw/qa/extras/layout/data/tdf104797.docx b/sw/qa/extras/layout/data/tdf104797.docx
new file mode 100644
index 000000000000..6e52190ce671
--- /dev/null
+++ b/sw/qa/extras/layout/data/tdf104797.docx
diff --git a/sw/qa/extras/layout/layout2.cxx b/sw/qa/extras/layout/layout2.cxx
index 9f765b1452c7..9eee8228ea8d 100644
--- a/sw/qa/extras/layout/layout2.cxx
+++ b/sw/qa/extras/layout/layout2.cxx
@@ -330,6 +330,26 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testRedlineMoving)
     assertXPath(pXmlDoc, "/metafile/push/push/push/font[@color='#008000']", 11);
 }
 
+CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testRedlineMovingDOCX)
+{
+    SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf104797.docx");
+    SwDocShell* pShell = pDoc->GetDocShell();
+
+    SwEditShell* const pEditShell(pDoc->GetEditShell());
+    // This was 2 (moveFrom and moveTo joined other redlines)
+    CPPUNIT_ASSERT_EQUAL(static_cast<SwRedlineTable::size_type>(6), pEditShell->GetRedlineCount());
+
+    // Dump the rendering of the first page as an XML file.
+    std::shared_ptr<GDIMetaFile> xMetaFile = pShell->GetPreviewMetaFile();
+    MetafileXmlDump dumper;
+    xmlDocUniquePtr pXmlDoc = dumpAndParse(dumper, *xMetaFile);
+    CPPUNIT_ASSERT(pXmlDoc);
+
+    // text colors show moved text
+    // These were 0 (other color, not COL_GREEN, color of the tracked text movement)
+    assertXPath(pXmlDoc, "/metafile/push/push/push/textcolor[@color='#008000']", 6);
+}
+
 CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testTdf145225_RedlineMovingWithBadInsertion)
 {
     SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf42748.fodt");
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
index d79ebd7a2113..8cb43ffe3e2c 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
@@ -896,7 +896,25 @@ DECLARE_OOXMLEXPORT_TEST(testTdf104797, "tdf104797.docx")
     CPPUNIT_ASSERT(hasProperty(getRun(getParagraph(2), 3), "RedlineType"));
     CPPUNIT_ASSERT_EQUAL(OUString("Insert"),getProperty<OUString>(getRun(getParagraph(2), 3), "RedlineType"));
     CPPUNIT_ASSERT_EQUAL(true,getProperty<bool>(getRun(getParagraph(2), 3), "IsStart"));
-    CPPUNIT_ASSERT_EQUAL( OUString( " Will this sentence be duplicated ADDED STUFF?" ), getRun( getParagraph( 2 ), 4 )->getString());
+
+    if (mbExported)
+    {
+        // TODO fix export of moved text
+        CPPUNIT_ASSERT_EQUAL( OUString( " Will this sentence be duplicated ADDED STUFF?" ), getRun( getParagraph( 2 ), 4 )->getString());
+    }
+    else
+    {
+        CPPUNIT_ASSERT_EQUAL( OUString( " " ), getRun( getParagraph( 2 ), 4 )->getString());
+        CPPUNIT_ASSERT_EQUAL( OUString( "" ), getRun( getParagraph( 2 ), 5 )->getString());
+        CPPUNIT_ASSERT(hasProperty(getRun(getParagraph(2), 5), "RedlineType"));
+        CPPUNIT_ASSERT_EQUAL(OUString("Insert"),getProperty<OUString>(getRun(getParagraph(2), 5), "RedlineType"));
+        CPPUNIT_ASSERT_EQUAL( OUString( "" ), getRun( getParagraph( 2 ), 6 )->getString());
+        CPPUNIT_ASSERT(hasProperty(getRun(getParagraph(2), 6), "RedlineType"));
+        CPPUNIT_ASSERT_EQUAL(true,getProperty<bool>(getRun(getParagraph(2), 6), "IsStart"));
+        CPPUNIT_ASSERT_EQUAL( OUString( "Will this sentence be duplicated" ), getRun( getParagraph( 2 ), 7 )->getString());
+        CPPUNIT_ASSERT_EQUAL( OUString( " ADDED STUFF" ), getRun( getParagraph( 2 ), 10 )->getString());
+        CPPUNIT_ASSERT_EQUAL( OUString( "?" ), getRun( getParagraph( 2 ), 13 )->getString());
+    }
 }
 
 DECLARE_OOXMLEXPORT_TEST(testTdf143510, "TC-table-DnD-move.docx")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
index 7d6e0583d312..86473c120cbb 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
@@ -767,16 +767,20 @@ DECLARE_OOXMLEXPORT_TEST(testTdf123460, "tdf123460.docx")
     CPPUNIT_ASSERT_EQUAL(OUString("Delete"),getProperty<OUString>(getRun(getParagraph(2), 1), "RedlineType"));
     CPPUNIT_ASSERT_EQUAL(true, getRun( getParagraph( 2 ), 2 )->getString().endsWith("tellus."));
     CPPUNIT_ASSERT_EQUAL( OUString( "" ), getRun( getParagraph( 2 ), 3 )->getString());
-    bool bCaught = false;
-    try
-    {
-        getRun( getParagraph( 2 ), 4 );
-    }
-    catch (container::NoSuchElementException&)
+    if (mbExported)
     {
-        bCaught = true;
+        // TODO fix export of moved text
+        bool bCaught = false;
+        try
+        {
+            getRun( getParagraph( 2 ), 4 );
+        }
+        catch (container::NoSuchElementException&)
+        {
+            bCaught = true;
+        }
+        CPPUNIT_ASSERT_EQUAL(true, bCaught);
     }
-    CPPUNIT_ASSERT_EQUAL(true, bCaught);
 }
 
 //tdf#125298: fix charlimit restrictions in bookmarknames and field references if they contain non-ascii characters
diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx b/sw/qa/extras/uiwriter/uiwriter2.cxx
index 9666efe9a49e..9797de01576f 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -3663,6 +3663,13 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf125916_redline_restart_numbering)
 {
     SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf125916.docx");
 
+    // moveFrom/moveTo are imported as separated redlines from fixing tdf#145718.
+    // Accept the first inline moveFrom redline before accepting the remaining ones
+    // to leave a paragraph long deletion to test the fix for tdf#125916.
+    SwEditShell* const pEditShell(pDoc->GetEditShell());
+    CPPUNIT_ASSERT(pEditShell->GetRedlineCount() > 0);
+    pEditShell->AcceptRedline(0);
+
     IDocumentRedlineAccess& rIDRA(pDoc->getIDocumentRedlineAccess());
     rIDRA.AcceptAllRedline(true);
 
diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx
index 86bb73039577..5211dff34147 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -1320,7 +1320,9 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall
             switch( pRedl->GetType() )
             {
             case RedlineType::Insert:
-                if( pRedl->IsOwnRedline( *pNewRedl ) )
+                if( pRedl->IsOwnRedline( *pNewRedl ) &&
+                    // don't join inserted characters with moved text
+                    !pRedl->IsMoved() )
                 {
                     bool bDelete = false;
 
@@ -1395,6 +1397,9 @@ DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCall
                         delete pNewRedl;
                         pNewRedl = nullptr;
                         bCompress = true;
+
+                        // set IsMoved checking nearby redlines
+                        maRedlineTable.isMoved(n);
                     }
                 }
                 else if( SwComparePosition::Inside == eCmpPos )
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index 36501906a641..732d92a40246 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -435,6 +435,10 @@ bool SwRedlineTable::Insert(SwRangeRedline*& p)
         std::pair<vector_type::const_iterator, bool> rv = maVector.insert( p );
         size_type nP = rv.first - begin();
         LOKRedlineNotification(RedlineNotification::Add, p);
+
+        // set IsMoved checking nearby redlines
+        isMoved(nP);
+
         p->CallDisplayFunc(nP);
         if (rv.second)
             CheckOverlapping(rv.first);
@@ -769,7 +773,7 @@ const SwRangeRedline* SwRedlineTable::FindAtPosition( const SwPosition& rSttPos,
 bool SwRedlineTable::isMoved( size_type rPos ) const
 {
     auto constexpr nLookahead = 20;
-    const SwRangeRedline* pRedline = (*this)[ rPos ];
+    SwRangeRedline* pRedline = (*this)[ rPos ];
 
     // set redline type of the searched pair
     RedlineType nPairType = pRedline->GetType();
@@ -800,7 +804,7 @@ bool SwRedlineTable::isMoved( size_type rPos ) const
     rPos = rPos > nLookahead ? rPos - nLookahead : 0;
     for ( ; rPos < nEnd ; ++rPos )
     {
-        const SwRangeRedline* pPair = (*this)[ rPos ];
+        SwRangeRedline* pPair = (*this)[ rPos ];
         // TODO handle also Show Changes in Margin mode
         if ( pPair->HasMark() && pPair->IsVisible() )
         {
@@ -810,6 +814,9 @@ bool SwRedlineTable::isMoved( size_type rPos ) const
                 abs(pRedline->GetText().getLength() - pPair->GetText().getLength()) <= 2 &&
                 sTrimmed == pPair->GetText().trim() )
             {
+                pRedline->SetMoved();
+                pPair->SetMoved();
+                pPair->InvalidateRange(SwRangeRedline::Invalidation::Remove);
                 return true;
             }
         }
@@ -1073,6 +1080,7 @@ SwRangeRedline::SwRangeRedline(RedlineType eTyp, const SwPaM& rPam )
 {
     m_bDelLastPara = false;
     m_bIsVisible = true;
+    m_bIsMoved = false;
     if( !rPam.HasMark() )
         DeleteMark();
 }
@@ -1085,6 +1093,7 @@ SwRangeRedline::SwRangeRedline( const SwRedlineData& rData, const SwPaM& rPam )
 {
     m_bDelLastPara = false;
     m_bIsVisible = true;
+    m_bIsMoved = false;
     if( !rPam.HasMark() )
         DeleteMark();
 }
@@ -1097,6 +1106,7 @@ SwRangeRedline::SwRangeRedline( const SwRedlineData& rData, const SwPosition& rP
 {
     m_bDelLastPara = false;
     m_bIsVisible = true;
+    m_bIsMoved = false;
 }
 
 SwRangeRedline::SwRangeRedline( const SwRangeRedline& rCpy )
@@ -1107,6 +1117,9 @@ SwRangeRedline::SwRangeRedline( const SwRangeRedline& rCpy )
 {
     m_bDelLastPara = false;
     m_bIsVisible = true;
+    // inserting characters within a moved text must keep
+    // IsMoved bit in the second part of the split redline
+    m_bIsMoved = rCpy.IsMoved();
     if( !rCpy.HasMark() )
         DeleteMark();
 }
@@ -1827,7 +1840,8 @@ void SwRangeRedline::SetContentIdx( const SwNodeIndex* pIdx )
 bool SwRangeRedline::CanCombine( const SwRangeRedline& rRedl ) const
 {
     return  IsVisible() && rRedl.IsVisible() &&
-            m_pRedlineData->CanCombine( *rRedl.m_pRedlineData );
+            m_pRedlineData->CanCombine( *rRedl.m_pRedlineData ) &&
+            !IsMoved() && !rRedl.IsMoved();
 }
 
 void SwRangeRedline::PushData( const SwRangeRedline& rRedl, bool bOwnAsNext )
diff --git a/sw/source/core/text/redlnitr.cxx b/sw/source/core/text/redlnitr.cxx
index f843ef4da6b7..3613469d7703 100644
--- a/sw/source/core/text/redlnitr.cxx
+++ b/sw/source/core/text/redlnitr.cxx
@@ -735,7 +735,7 @@ short SwRedlineItr::Seek(SwFont& rFnt,
                     SfxWhichIter aIter( *m_pSet );
 
                     // moved text: dark green with double underline or strikethrough
-                    if ( rTable.isMoved( m_nAct ) )
+                    if ( pRed->IsMoved() )
                     {
                         m_pSet->Put(SvxColorItem( COL_GREEN, RES_CHRATR_COLOR ));
                         if (SfxItemState::SET == m_pSet->GetItemState(RES_CHRATR_CROSSEDOUT, true))
diff --git a/sw/source/core/text/txtfld.cxx b/sw/source/core/text/txtfld.cxx
index a34ccf863600..fc25dc4e9880 100644
--- a/sw/source/core/text/txtfld.cxx
+++ b/sw/source/core/text/txtfld.cxx
@@ -554,7 +554,7 @@ static const SwRangeRedline* lcl_GetRedlineAtNodeInsertionOrDeletion( const SwTe
                 const SwPosition *pRStt = pTmp->Start(), *pREnd = pTmp->End();
                 if( pRStt->nNode <= nNdIdx && pREnd->nNode > nNdIdx )
                 {
-                    bIsMoved = rTable.isMoved(nRedlPos);
+                    bIsMoved = pTmp->IsMoved();
                     return pTmp;
                 }
             }
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index 8343e1f37a3b..9f6884d7d087 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -1367,6 +1367,12 @@ void makeRedline( SwPaM const & rPaM,
     }
 
     SwRangeRedline* pRedline = new SwRangeRedline( aRedlineData, rPaM );
+
+    // set IsMoved bit of the redline to show and handle moved text
+    bool bIsMoved;
+    if( (aPropMap.getValue("RedlineMoved") >>= bIsMoved) && bIsMoved )
+        pRedline->SetMoved();
+
     RedlineFlags nPrevMode = rRedlineAccess.GetRedlineFlags( );
     // xRedlineExtraData is copied here
     pRedline->SetExtraData( xRedlineExtraData.get() );
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index c42a085bfa87..b52969e300a7 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -3039,6 +3039,7 @@ void DomainMapper_Impl::CreateRedline(uno::Reference<text::XTextRange> const& xR
     if ( !pRedline )
         return;
 
+    bool bRedlineMoved = false;
     try
     {
         OUString sType;
@@ -3048,10 +3049,13 @@ void DomainMapper_Impl::CreateRedline(uno::Reference<text::XTextRange> const& xR
             sType = getPropertyName( PROP_FORMAT );
             break;
         case XML_moveTo:
+            bRedlineMoved = true;
+            [[fallthrough]];
         case XML_ins:
             sType = getPropertyName( PROP_INSERT );
             break;
         case XML_moveFrom:
+            bRedlineMoved = true;
             m_pParaMarkerRedlineMoveFrom = pRedline.get();
             [[fallthrough]];
         case XML_del:
@@ -3063,7 +3067,7 @@ void DomainMapper_Impl::CreateRedline(uno::Reference<text::XTextRange> const& xR
         default:
             throw lang::IllegalArgumentException("illegal redline token type", nullptr, 0);
         }
-        beans::PropertyValues aRedlineProperties( 3 );
+        beans::PropertyValues aRedlineProperties( 4 );
         beans::PropertyValue * pRedlineProperties = aRedlineProperties.getArray(  );
         pRedlineProperties[0].Name = getPropertyName( PROP_REDLINE_AUTHOR );
         pRedlineProperties[0].Value <<= pRedline->m_sAuthor;
@@ -3071,6 +3075,9 @@ void DomainMapper_Impl::CreateRedline(uno::Reference<text::XTextRange> const& xR
         pRedlineProperties[1].Value <<= ConversionHelper::ConvertDateStringToDateTime( pRedline->m_sDate );
         pRedlineProperties[2].Name = getPropertyName( PROP_REDLINE_REVERT_PROPERTIES );
         pRedlineProperties[2].Value <<= pRedline->m_aRevertProperties;
+        pRedlineProperties[3].Name = "RedlineMoved";
+        pRedlineProperties[3].Value <<= bRedlineMoved;
+
         if (!m_bIsActualParagraphFramed)
         {
             uno::Reference < text::XRedline > xRedline( xRange, uno::UNO_QUERY_THROW );