sw comments on frames: fix DOC handling

The export part didn't use HasFlysAt() to decide what to do with empty
annotation marks.

Regarding the import side, the only problem was that the start of the
annotation range wasn't adjusted to cover the comment anchor, similar to
what the UI and the UNO API already does.

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

6 files changed, 53 insertions, 8 deletions

diff --git a/sw/qa/extras/ww8export/data/image-comment-at-char.doc b/sw/qa/extras/ww8export/data/image-comment-at-char.doc
new file mode 100644
index 000000000000..e0d5e2cba456
--- /dev/null
+++ b/sw/qa/extras/ww8export/data/image-comment-at-char.doc
diff --git a/sw/qa/extras/ww8export/ww8export3.cxx b/sw/qa/extras/ww8export/ww8export3.cxx
index fcf5fc44379f..20de0969805a 100644
--- a/sw/qa/extras/ww8export/ww8export3.cxx
+++ b/sw/qa/extras/ww8export/ww8export3.cxx
@@ -259,6 +259,23 @@ DECLARE_WW8EXPORT_TEST(testTdf118375export, "tdf118375_240degClockwise.doc")
     CPPUNIT_ASSERT_DOUBLES_EQUAL(1152.0, static_cast<double>(nPosY), 1.0);
 }
 
+DECLARE_WW8EXPORT_TEST(testImageCommentAtChar, "image-comment-at-char.doc")
+{
+    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/basflt/fltshell.cxx b/sw/source/filter/basflt/fltshell.cxx
index a28cf6732597..62e5920fc2bb 100644
--- a/sw/source/filter/basflt/fltshell.cxx
+++ b/sw/source/filter/basflt/fltshell.cxx
@@ -662,6 +662,17 @@ void SwFltControlStack::SetAttrInDoc(const SwPosition& rTmpPos,
                     if (pPostIt)
                     {
                         assert(pPostIt->GetName().isEmpty());
+
+                        if (!aRegion.HasMark())
+                        {
+                            // Annotation range was found in the file, but start/end is the same,
+                            // pointing after the postit placeholder (see assert above).
+                            // Adjust the start of the range to actually cover the comment, similar
+                            // to what the UI and the UNO API does.
+                            aRegion.SetMark();
+                            --aRegion.Start()->nContent;
+                        }
+
                         pDoc->getIDocumentMarkAccess()->makeAnnotationMark(aRegion, OUString());
                     }
                     else
diff --git a/sw/source/filter/ww8/wrtw8sty.cxx b/sw/source/filter/ww8/wrtw8sty.cxx
index c65de3898ff6..35ecc773eb05 100644
--- a/sw/source/filter/ww8/wrtw8sty.cxx
+++ b/sw/source/filter/ww8/wrtw8sty.cxx
@@ -1962,9 +1962,20 @@ WW8_Annotation::WW8_Annotation(const SwRedlineData* pRedline)
     maDateTime = pRedline->GetTimeStamp();
 }
 
-void WW8_WrPlcAnnotations::AddRangeStartPosition(const OUString& rName, WW8_CP nStartCp)
+bool WW8_Annotation::HasRange() const
 {
-    m_aRangeStartPositions[rName] = nStartCp;
+    if (m_nRangeStart != m_nRangeEnd)
+    {
+        return true;
+    }
+
+    return !m_bIgnoreEmpty;
+}
+
+void WW8_WrPlcAnnotations::AddRangeStartPosition(const OUString& rName, WW8_CP nStartCp,
+                                                 bool bIgnoreEmpty)
+{
+    m_aRangeStartPositions[rName] = std::make_pair(nStartCp, bIgnoreEmpty);
 }
 
 void WW8_WrPlcAnnotations::Append( WW8_CP nCp, const SwPostItField *pPostIt )
@@ -1973,7 +1984,9 @@ void WW8_WrPlcAnnotations::Append( WW8_CP nCp, const SwPostItField *pPostIt )
     WW8_Annotation* p;
     if( m_aRangeStartPositions.find(pPostIt->GetName()) != m_aRangeStartPositions.end() )
     {
-        p = new WW8_Annotation(pPostIt, m_aRangeStartPositions[pPostIt->GetName()], nCp);
+        auto [nStartCp, bIgnoreEmpty] = m_aRangeStartPositions[pPostIt->GetName()];
+        p = new WW8_Annotation(pPostIt, nStartCp, nCp);
+        p->m_bIgnoreEmpty = bIgnoreEmpty;
         m_aRangeStartPositions.erase(pPostIt->GetName());
     }
     else
@@ -2178,7 +2191,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                     const WW8_Annotation& rAtn = *static_cast<const WW8_Annotation*>(aContent[i]);
                     aStrArr.emplace_back(rAtn.msOwner,rAtn.m_sInitials);
                     // record start and end positions for ranges
-                    if( rAtn.m_nRangeStart != rAtn.m_nRangeEnd )
+                    if (rAtn.HasRange())
                     {
                         aRangeStartPos.emplace_back(rAtn.m_nRangeStart, nIdx);
                         aRangeEndPos.emplace_back(rAtn.m_nRangeEnd, nIdx);
@@ -2403,7 +2416,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                 SwWW8Writer::WriteShort( *rWrt.pTableStrm, nFndPos );
                 SwWW8Writer::WriteShort( *rWrt.pTableStrm, 0 );
                 SwWW8Writer::WriteShort( *rWrt.pTableStrm, 0 );
-                if( rAtn.m_nRangeStart != rAtn.m_nRangeEnd )
+                if (rAtn.HasRange())
                 {
                     SwWW8Writer::WriteLong( *rWrt.pTableStrm, nlTag );
                     ++nlTag;
diff --git a/sw/source/filter/ww8/wrtww8.cxx b/sw/source/filter/ww8/wrtww8.cxx
index 1cabdca7b9f0..8f78c2382b1d 100644
--- a/sw/source/filter/ww8/wrtww8.cxx
+++ b/sw/source/filter/ww8/wrtww8.cxx
@@ -1490,7 +1490,8 @@ void WW8Export::AppendAnnotationMarks(const SwWW8AttrIter& rAttrs, sal_Int32 nCu
             const sal_Int32 nStart = pMark->GetMarkStart().nContent.GetIndex();
             if (nStart == nCurrentPos)
             {
-                m_pAtn->AddRangeStartPosition(pMark->GetName(), Fc2Cp(Strm().Tell()));
+                m_pAtn->AddRangeStartPosition(pMark->GetName(), Fc2Cp(Strm().Tell()),
+                                              !rAttrs.HasFlysAt(nCurrentPos));
             }
         }
     }
diff --git a/sw/source/filter/ww8/wrtww8.hxx b/sw/source/filter/ww8/wrtww8.hxx
index 6a01f61ed3c0..1ee0c86581bf 100644
--- a/sw/source/filter/ww8/wrtww8.hxx
+++ b/sw/source/filter/ww8/wrtww8.hxx
@@ -1230,8 +1230,11 @@ struct WW8_Annotation
     OUString m_sInitials;
     DateTime maDateTime;
     WW8_CP m_nRangeStart, m_nRangeEnd;
+    bool m_bIgnoreEmpty = true;
     WW8_Annotation(const SwPostItField* pPostIt, WW8_CP nRangeStart, WW8_CP nRangeEnd);
     explicit WW8_Annotation(const SwRedlineData* pRedline);
+    /// An annotation has a range if start != end or the m_bIgnoreEmpty flag is cleared.
+    bool HasRange() const;
 };
 
 class WW8_WrPlcAnnotations : public WW8_WrPlcSubDoc  // double Plc for Postits
@@ -1241,12 +1244,12 @@ private:
     WW8_WrPlcAnnotations& operator=(WW8_WrPlcAnnotations const &) = delete;
     std::set<const SwRedlineData*> maProcessedRedlines;
 
-    std::map<const OUString, WW8_CP> m_aRangeStartPositions;
+    std::map<const OUString, std::pair<WW8_CP, bool>> m_aRangeStartPositions;
 public:
     WW8_WrPlcAnnotations() {}
     virtual ~WW8_WrPlcAnnotations() override;
 
-    void AddRangeStartPosition(const OUString& rName, WW8_CP nStartCp);
+    void AddRangeStartPosition(const OUString& rName, WW8_CP nStartCp, bool bIgnoreEmpty);
     void Append( WW8_CP nCp, const SwPostItField* pPostIt );
     void Append( WW8_CP nCp, const SwRedlineData* pRedLine );
     bool IsNewRedlineComment( const SwRedlineData* pRedLine );