sw comments on frames: fix DOCX handling

We used to ignore annotation marks which just cover the comment anchor
since commit fff019debf14a0bf8cd358591a686191347f1542 (MSWordExportBase:
ignore empty annotation marks, 2014-09-17), but this means comments on
images are lost.

Pass around SwWW8AttrIter, so we can decide if we have a relevant
at-char anchored frame in MSWordExportBase::GetAnnotationMarks(),
without iterating over all frames in the document, which would be slow
for large documents.

Regarding the import side, the only problem was that the empty comment
range resulted in a loss of annotation marks; fix that by using a marker
while inserting.

Change-Id: I385677d74423bc05824dac4a12d1a991bb3983c4
Reviewed-on: https://gerrit.libreoffice.org/74996
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

9 files changed, 66 insertions, 20 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/image-comment-at-char.docx b/sw/qa/extras/ooxmlexport/data/image-comment-at-char.docx
new file mode 100644
index 000000000000..677464de49f7
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/image-comment-at-char.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
index d377f9b14449..d09a367f2536 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
@@ -432,6 +432,23 @@ DECLARE_OOXMLEXPORT_TEST(testTdf125518, "tdf125518.odt")
     CPPUNIT_ASSERT_EQUAL(OUString("Object3"), anchorName);
 }
 
+DECLARE_OOXMLEXPORT_TEST(testImageCommentAtChar, "image-comment-at-char.docx")
+{
+    uno::Reference<text::XTextRange> xPara = getParagraph(1);
+    CPPUNIT_ASSERT_EQUAL(OUString("Text"),
+                         getProperty<OUString>(getRun(xPara, 1), "TextPortionType"));
+    // Without the accompanying fix in place, this test would have failed with 'Expected:
+    // Annotation; Actual: Frame', i.e. the comment start before the image was lost.
+    CPPUNIT_ASSERT_EQUAL(OUString("Annotation"),
+                         getProperty<OUString>(getRun(xPara, 2), "TextPortionType"));
+    CPPUNIT_ASSERT_EQUAL(OUString("Frame"),
+                         getProperty<OUString>(getRun(xPara, 3), "TextPortionType"));
+    CPPUNIT_ASSERT_EQUAL(OUString("AnnotationEnd"),
+                         getProperty<OUString>(getRun(xPara, 4), "TextPortionType"));
+    CPPUNIT_ASSERT_EQUAL(OUString("Text"),
+                         getProperty<OUString>(getRun(xPara, 5), "TextPortionType"));
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/ww8/docxexport.cxx b/sw/source/filter/ww8/docxexport.cxx
index b9870a93e30e..eb10711b6e28 100644
--- a/sw/source/filter/ww8/docxexport.cxx
+++ b/sw/source/filter/ww8/docxexport.cxx
@@ -189,13 +189,13 @@ void DocxExport::AppendBookmark( const OUString& rName )
     m_pAttrOutput->WriteBookmarks_Impl( aStarts, aEnds );
 }
 
-void DocxExport::AppendAnnotationMarks( const SwTextNode& rNode, sal_Int32 nCurrentPos, sal_Int32 nLen )
+void DocxExport::AppendAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen )
 {
     std::vector< OUString > aStarts;
     std::vector< OUString > aEnds;
 
     IMarkVector aMarks;
-    if ( GetAnnotationMarks( rNode, nCurrentPos, nCurrentPos + nLen, aMarks ) )
+    if (GetAnnotationMarks(rAttrs, nCurrentPos, nCurrentPos + nLen, aMarks))
     {
         for ( IMark* pMark : aMarks )
         {
diff --git a/sw/source/filter/ww8/docxexport.hxx b/sw/source/filter/ww8/docxexport.hxx
index 0424f2619e75..fb04593aea9c 100644
--- a/sw/source/filter/ww8/docxexport.hxx
+++ b/sw/source/filter/ww8/docxexport.hxx
@@ -147,7 +147,7 @@ public:
 
     virtual void AppendBookmark( const OUString& rName ) override;
 
-    virtual void AppendAnnotationMarks( const SwTextNode& rNode, sal_Int32 nCurrentPos, sal_Int32 nLen ) override;
+    virtual void AppendAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen ) override;
 
     virtual void ExportGrfBullet(const SwTextNode&) override;
 
diff --git a/sw/source/filter/ww8/rtfexport.cxx b/sw/source/filter/ww8/rtfexport.cxx
index 4c8ed0281537..d06f1259c7ce 100644
--- a/sw/source/filter/ww8/rtfexport.cxx
+++ b/sw/source/filter/ww8/rtfexport.cxx
@@ -150,14 +150,14 @@ void RtfExport::AppendBookmark(const OUString& rName)
     m_pAttrOutput->WriteBookmarks_Impl(aStarts, aEnds);
 }
 
-void RtfExport::AppendAnnotationMarks(const SwTextNode& rNode, sal_Int32 nCurrentPos,
+void RtfExport::AppendAnnotationMarks(const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos,
                                       sal_Int32 nLen)
 {
     std::vector<OUString> aStarts;
     std::vector<OUString> aEnds;
 
     IMarkVector aMarks;
-    if (GetAnnotationMarks(rNode, nCurrentPos, nCurrentPos + nLen, aMarks))
+    if (GetAnnotationMarks(rAttrs, nCurrentPos, nCurrentPos + nLen, aMarks))
     {
         for (const auto& pMark : aMarks)
         {
diff --git a/sw/source/filter/ww8/rtfexport.hxx b/sw/source/filter/ww8/rtfexport.hxx
index b95dec3fac55..321b2168ef37 100644
--- a/sw/source/filter/ww8/rtfexport.hxx
+++ b/sw/source/filter/ww8/rtfexport.hxx
@@ -73,7 +73,7 @@ public:
 
     void AppendBookmark(const OUString& rName) override;
 
-    void AppendAnnotationMarks(const SwTextNode& rNode, sal_Int32 nCurrentPos,
+    void AppendAnnotationMarks(const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos,
                                sal_Int32 nLen) override;
 
     //For i120928,add an interface to export graphic of bullet
diff --git a/sw/source/filter/ww8/wrtw8nds.cxx b/sw/source/filter/ww8/wrtw8nds.cxx
index b1da5bc210e2..19b901fe0e31 100644
--- a/sw/source/filter/ww8/wrtw8nds.cxx
+++ b/sw/source/filter/ww8/wrtw8nds.cxx
@@ -563,6 +563,21 @@ bool SwWW8AttrIter::IsAnchorLinkedToThisNode( sal_uLong nNodePos )
     return false ;
 }
 
+bool SwWW8AttrIter::HasFlysAt(sal_Int32 nSwPos) const
+{
+    for (const auto& rFly : maFlyFrames)
+    {
+        const SwPosition& rAnchor = rFly.GetPosition();
+        const sal_Int32 nPos = rAnchor.nContent.GetIndex();
+        if (nPos == nSwPos)
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 FlyProcessingState SwWW8AttrIter::OutFlys(sal_Int32 nSwPos)
 {
     // collection point to first gather info about all of the potentially linked textboxes: to be analyzed later.
@@ -1835,7 +1850,7 @@ sal_Int32 MSWordExportBase::GetNextPos( SwWW8AttrIter const * aAttrIter, const S
     {
         GetSortedBookmarks( rNode, nCurrentPos, nNextBookmark - nCurrentPos );
         NearestBookmark( nNextBookmark, nCurrentPos, false );
-        GetSortedAnnotationMarks( rNode, nCurrentPos, nNextAnnotationMark - nCurrentPos );
+        GetSortedAnnotationMarks(*aAttrIter, nCurrentPos, nNextAnnotationMark - nCurrentPos);
         NearestAnnotationMark( nNextAnnotationMark, nCurrentPos, false );
     }
     return std::min( nNextPos, std::min( nNextBookmark, nNextAnnotationMark ) );
@@ -1889,11 +1904,11 @@ bool MSWordExportBase::GetBookmarks( const SwTextNode& rNd, sal_Int32 nStt,
     return ( !rArr.empty() );
 }
 
-bool MSWordExportBase::GetAnnotationMarks( const SwTextNode& rNd, sal_Int32 nStt,
+bool MSWordExportBase::GetAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nStt,
                     sal_Int32 nEnd, IMarkVector& rArr )
 {
     IDocumentMarkAccess* const pMarkAccess = m_pDoc->getIDocumentMarkAccess();
-    sal_uLong nNd = rNd.GetIndex( );
+    sal_uLong nNd = rAttrs.GetNode().GetIndex();
 
     const sal_Int32 nMarks = pMarkAccess->getAnnotationMarksCount();
     for ( sal_Int32 i = 0; i < nMarks; i++ )
@@ -1916,6 +1931,16 @@ bool MSWordExportBase::GetAnnotationMarks( const SwTextNode& rNd, sal_Int32 nStt
             // comment field, so ignore the annotation mark itself.
             bool bSingleChar = pMark->GetMarkStart().nNode == pMark->GetMarkEnd().nNode && nBStart + 1 == nBEnd;
 
+            if (bSingleChar)
+            {
+                if (rAttrs.HasFlysAt(nBStart))
+                {
+                    // There is content (an at-char anchored frame) between the annotation mark
+                    // start/end, so still emit range start/end.
+                    bSingleChar = false;
+                }
+            }
+
             if ( ( bIsStartOk || bIsEndOk ) && !bSingleChar )
             {
                 rArr.push_back( pMark );
@@ -1998,10 +2023,10 @@ void MSWordExportBase::NearestAnnotationMark( sal_Int32& rNearest, const sal_Int
     }
 }
 
-void MSWordExportBase::GetSortedAnnotationMarks( const SwTextNode& rNode, sal_Int32 nCurrentPos, sal_Int32 nLen )
+void MSWordExportBase::GetSortedAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen )
 {
     IMarkVector aMarksStart;
-    if ( GetAnnotationMarks( rNode, nCurrentPos, nCurrentPos + nLen, aMarksStart ) )
+    if (GetAnnotationMarks(rAttrs, nCurrentPos, nCurrentPos + nLen, aMarksStart))
     {
         IMarkVector aSortedEnd;
         IMarkVector aSortedStart;
@@ -2011,6 +2036,7 @@ void MSWordExportBase::GetSortedAnnotationMarks( const SwTextNode& rNode, sal_In
             const sal_Int32 nStart = pMark->GetMarkStart().nContent.GetIndex();
             const sal_Int32 nEnd = pMark->GetMarkEnd().nContent.GetIndex();
 
+            const SwTextNode& rNode = rAttrs.GetNode();
             if ( nStart > nCurrentPos && ( pMark->GetMarkStart().nNode == rNode.GetIndex()) )
                 aSortedStart.push_back( pMark );
 
@@ -2280,7 +2306,7 @@ void MSWordExportBase::OutputTextNode( SwTextNode& rNode )
             // Append bookmarks in this range after flys, exclusive of final
             // position of this range
             AppendBookmarks( rNode, nCurrentPos, nNextAttr - nCurrentPos );
-            AppendAnnotationMarks( rNode, nCurrentPos, nNextAttr - nCurrentPos );
+            AppendAnnotationMarks(aAttrIter, nCurrentPos, nNextAttr - nCurrentPos);
 
             // At the moment smarttags are only written for paragraphs, at the
             // beginning of the paragraph.
@@ -2464,7 +2490,7 @@ void MSWordExportBase::OutputTextNode( SwTextNode& rNode )
                         nStateOfFlyFrame = aAttrIter.OutFlys( nEnd );
                         // insert final bookmarks if any before CR and after flys
                         AppendBookmarks( rNode, nEnd, 1 );
-                        AppendAnnotationMarks( rNode, nEnd, 1 );
+                        AppendAnnotationMarks(aAttrIter, nEnd, 1);
                         if ( pTOXSect )
                         {
                             m_aCurrentCharPropStarts.pop();
@@ -2518,7 +2544,7 @@ void MSWordExportBase::OutputTextNode( SwTextNode& rNode )
                     nStateOfFlyFrame = aAttrIter.OutFlys( nEnd );
                     // insert final bookmarks if any before CR and after flys
                     AppendBookmarks( rNode, nEnd, 1 );
-                    AppendAnnotationMarks( rNode, nEnd, 1 );
+                    AppendAnnotationMarks(aAttrIter, nEnd, 1);
                     WriteCR( pTextNodeInfoInner );
                     // #i120928 - position of the bullet's graphic is at end of doc
                     if (bLastCR && (!bExported))
diff --git a/sw/source/filter/ww8/wrtww8.cxx b/sw/source/filter/ww8/wrtww8.cxx
index 7773e25549ce..1cabdca7b9f0 100644
--- a/sw/source/filter/ww8/wrtww8.cxx
+++ b/sw/source/filter/ww8/wrtww8.cxx
@@ -1480,10 +1480,10 @@ void WW8Export::AppendBookmarks( const SwTextNode& rNd, sal_Int32 nCurrentPos, s
     }
 }
 
-void WW8Export::AppendAnnotationMarks(const SwTextNode& rNode, sal_Int32 nCurrentPos, sal_Int32 nLen)
+void WW8Export::AppendAnnotationMarks(const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen)
 {
     IMarkVector aMarks;
-    if (GetAnnotationMarks(rNode, nCurrentPos, nCurrentPos + nLen, aMarks))
+    if (GetAnnotationMarks(rAttrs, nCurrentPos, nCurrentPos + nLen, aMarks))
     {
         for (const sw::mark::IMark* pMark : aMarks)
         {
diff --git a/sw/source/filter/ww8/wrtww8.hxx b/sw/source/filter/ww8/wrtww8.hxx
index cc2255e78411..6a01f61ed3c0 100644
--- a/sw/source/filter/ww8/wrtww8.hxx
+++ b/sw/source/filter/ww8/wrtww8.hxx
@@ -688,7 +688,7 @@ public:
 
     virtual void AppendBookmark( const OUString& rName ) = 0;
 
-    virtual void AppendAnnotationMarks( const SwTextNode& rNd, sal_Int32 nCurrentPos, sal_Int32 nLen ) = 0;
+    virtual void AppendAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen ) = 0;
 
     virtual void AppendSmartTags(SwTextNode& /*rTextNode*/) { }
 
@@ -868,9 +868,9 @@ protected:
     ///
     void NearestAnnotationMark( sal_Int32& rNearest, const sal_Int32 nCurrentPos, bool bNextPositionOnly );
 
-    void GetSortedAnnotationMarks( const SwTextNode& rNd, sal_Int32 nCurrentPos, sal_Int32 nLen );
+    void GetSortedAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen );
 
-    bool GetAnnotationMarks( const SwTextNode& rNd, sal_Int32 nStt, sal_Int32 nEnd,
+    bool GetAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nStt, sal_Int32 nEnd,
             IMarkVector& rArr );
 
     const NfKeywordTable & GetNfKeywordTable();
@@ -1058,7 +1058,7 @@ public:
     virtual void AppendBookmark( const OUString& rName ) override;
     void AppendBookmarkEndWithCorrection( const OUString& rName );
 
-    virtual void AppendAnnotationMarks( const SwTextNode& rNd, sal_Int32 nCurrentPos, sal_Int32 nLen ) override;
+    virtual void AppendAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int32 nCurrentPos, sal_Int32 nLen ) override;
 
     virtual void AppendSmartTags(SwTextNode& rTextNode) override;
 
@@ -1518,6 +1518,7 @@ public:
     const SwRedlineData* GetParagraphLevelRedline( );
     const SwRedlineData* GetRunLevelRedline( sal_Int32 nPos );
     FlyProcessingState OutFlys(sal_Int32 nSwPos);
+    bool HasFlysAt(sal_Int32 nSwPos) const;
 
     sal_Int32 WhereNext() const { return nCurrentSwPos; }
     sal_uInt16 GetScript() const { return mnScript; }
@@ -1531,6 +1532,8 @@ public:
     bool IsAnchorLinkedToThisNode( sal_uLong nNodePos );
 
     void SplitRun( sal_Int32 nSplitEndPos );
+
+    const SwTextNode& GetNode() const { return rNd; }
 };
 
 /// Class to collect and output the styles table.