i#93570 DOC import: handle commented text ranges via RES_FLTR_ANNOTATIONMARK

And add a minimal reproducer that shows how the old way was broken.

Change-Id: Ic2dadf9905d603b0fd0573651b235ecd5dd70e71

7 files changed, 29 insertions, 156 deletions

diff --git a/sw/qa/extras/ww8export/data/commented-table.doc b/sw/qa/extras/ww8export/data/commented-table.doc
new file mode 100644
index 000000000000..049c7e3a4b2e
--- /dev/null
+++ b/sw/qa/extras/ww8export/data/commented-table.doc
diff --git a/sw/qa/extras/ww8export/ww8export.cxx b/sw/qa/extras/ww8export/ww8export.cxx
index 8a4596aa54a4..11cf7782ffd7 100644
--- a/sw/qa/extras/ww8export/ww8export.cxx
+++ b/sw/qa/extras/ww8export/ww8export.cxx
@@ -441,6 +441,18 @@ DECLARE_WW8EXPORT_TEST(testWw8Cjklist35, "cjklist35.doc")
     CPPUNIT_ASSERT_EQUAL(style::NumberingType::NUMBER_LOWER_ZH, numFormat);
 }
 
+DECLARE_WW8EXPORT_TEST(testCommentedTable, "commented-table.doc")
+{
+    // Document has a non-trivial commented text range, as the range contains a table.
+    uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xFieldsAccess(xTextFieldsSupplier->getTextFields());
+    uno::Reference<container::XEnumeration> xFields(xFieldsAccess->createEnumeration());
+    uno::Reference<text::XTextContent> xField(xFields->nextElement(), uno::UNO_QUERY);
+    // After first import, there was an off-by-one during import, so this was "efore.\nA1\nB1\nAfte". (Notice the additional "e" prefix.)
+    // After export and import, things got worse, this was "\nA1\nB1\nAfte".
+    CPPUNIT_ASSERT_EQUAL(OUString("fore.\nA1\nB1\nAfte"), xField->getAnchor()->getString());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/ww8/ww8par.cxx b/sw/source/filter/ww8/ww8par.cxx
index 7c42b694dc37..67aa9ab0e93f 100644
--- a/sw/source/filter/ww8/ww8par.cxx
+++ b/sw/source/filter/ww8/ww8par.cxx
@@ -2185,79 +2185,6 @@ long SwWW8ImplReader::Read_And(WW8PLCFManResult* pRes)
             sAuthor = *pA;
         else
             sAuthor = sInitials;
-
-        // If there is a bookmark tag, a text range should be commented.
-        sal_uInt32 nTagBkmk = SVBT32ToUInt32(pDescri->ITagBkmk);
-        if (nTagBkmk != 0xFFFFFFFF)
-        {
-            int nAtnIndex = GetAnnotationIndex(nTagBkmk);
-            if (nAtnIndex != -1)
-            {
-                WW8_CP nStart = GetAnnotationStart(nAtnIndex);
-                WW8_CP nEnd = GetAnnotationEnd(GetAnnotationEndIndex(nAtnIndex));
-                //It is unfortunately fragile and wrong to assume that two
-                //character positions in the original word document, which is
-                //what nStart and nEnd are, will equate to the same length in
-                //the destination writer document.
-                //
-                //Better would be, while writing the content into the writer
-                //document to store the equivalent writer document positions
-                //that relate to each annotation index as the parser passes
-                //those points.
-                sal_Int32 nLen = nEnd - nStart;
-                // the start and end positions are apparently stored in
-                // different arrays, so in an invalid file only one could exist
-                if(SAL_MAX_INT32 != nEnd && SAL_MAX_INT32 != nStart && nLen > 0)
-                {
-                    if (pPaM->GetPoint()->nContent.GetIndex() >= nLen)
-                    {
-                        pPaM->SetMark();
-                        pPaM->GetPoint()->nContent -= nLen;
-                    }
-                    else if (pPaM->GetPoint()->nNode.GetNode().IsTxtNode() )
-                    {
-                        pPaM->SetMark();
-                        nLen -= pPaM->GetPoint()->nContent.GetIndex();
-
-                        SwTxtNode* pTxtNode = 0;
-
-                        // Find first text node which is affected by the comment
-                        while (nLen > 0)
-                        {
-                            // Move to previous content node
-                            bool bSuccess = pPaM->Move(fnMoveBackward, fnGoNode);
-
-                            if (!bSuccess)
-                            {
-                                nLen = 0;
-                                break;
-                            }
-
-                            --nLen; // End line character
-
-                            SwNode& rNode = pPaM->GetPoint()->nNode.GetNode();
-
-                            // Subtract previous text node's length
-                            if (rNode.IsTxtNode())
-                            {
-                                pTxtNode = rNode.GetTxtNode();
-                                if (nLen < pTxtNode->Len())
-                                    break;
-                                else
-                                    nLen -= pTxtNode->Len();
-                            }
-                        }
-
-                        // Set position of the text range's first character
-                        if( pTxtNode )
-                        {
-                            pTxtNode->MakeStartIndex(&pPaM->GetPoint()->nContent);
-                            pPaM->GetPoint()->nContent += pTxtNode->Len() - nLen;
-                        }
-                    }
-                }
-            }
-        }
     }
 
     sal_uInt32 nDateTime = 0;
@@ -2285,15 +2212,8 @@ long SwWW8ImplReader::Read_And(WW8PLCFManResult* pRes)
     pCtrlStck->NewAttr(*aEnd.GetPoint(), SvxCharHiddenItem(false, RES_CHRATR_HIDDEN));
     rDoc.getIDocumentContentOperations().InsertPoolItem(aEnd, SwFmtFld(aPostIt), 0);
     pCtrlStck->SetAttr(*aEnd.GetPoint(), RES_CHRATR_HIDDEN);
-
-    // If this is a range, create the associated fieldmark.
-    if (pPaM->HasMark())
-    {
-        IDocumentMarkAccess* pMarksAccess = rDoc.getIDocumentMarkAccess();
-        pMarksAccess->makeAnnotationMark(*pPaM, aPostIt.GetName());
-        pPaM->Exchange();
-        pPaM->DeleteMark();
-    }
+    // If this is a range, make sure that it ends after the just inserted character, not before it.
+    pReffedStck->MoveAttrs(*aEnd.GetPoint());
 
     return 0;
 }
@@ -6010,66 +5930,6 @@ const OUString* SwWW8ImplReader::GetAnnotationAuthor(sal_uInt16 nIdx)
     return pRet;
 }
 
-int SwWW8ImplReader::GetAnnotationIndex(sal_uInt32 nTag)
-{
-    if (!mpAtnIndexes.get() && pWwFib->lcbSttbfAtnbkmk)
-    {
-        mpAtnIndexes.reset(new std::map<sal_uInt32, int>());
-        std::vector<OUString> aStrings;
-        std::vector<ww::bytes> aEntries;
-        WW8ReadSTTBF(!bVer67, *pTableStream, pWwFib->fcSttbfAtnbkmk, pWwFib->lcbSttbfAtnbkmk, sizeof(struct WW8_ATNBE), eStructCharSet, aStrings, &aEntries);
-        for (size_t i = 0; i < aStrings.size() && i < aEntries.size(); ++i)
-        {
-            ww::bytes aEntry = aEntries[i];
-            WW8_ATNBE* pAtnbeStruct = (WW8_ATNBE*)(&aEntry[0]);
-            mpAtnIndexes->insert(std::pair<sal_uInt32, int>(SVBT32ToUInt32(pAtnbeStruct->nTag), i));
-        }
-    }
-    if (mpAtnIndexes.get())
-    {
-        std::map<sal_uInt32, int>::iterator it = mpAtnIndexes->find(nTag);
-        if (it != mpAtnIndexes->end())
-            return it->second;
-    }
-    return -1;
-}
-
-sal_uInt16 SwWW8ImplReader::GetAnnotationEndIndex(sal_uInt16 nStart)
-{
-    WW8_CP nStartAkt;
-    void* p;
-    if (mpAtnStarts->GetData(nStart, nStartAkt, p) && p)
-    {
-        // p is an FBKF, and its first 2 bytes is the ibkl member, which is the end index.
-        return SVBT16ToShort(*static_cast<SVBT16*>(p));
-    }
-    return nStart;
-}
-
-WW8_CP SwWW8ImplReader::GetAnnotationStart(int nIndex)
-{
-    if (!mpAtnStarts.get() && pWwFib->lcbPlcfAtnbkf)
-        // A PLCFBKF is a PLC whose data elements are FBKF structures (4 bytes each).
-        mpAtnStarts.reset(new WW8PLCFspecial(pTableStream, pWwFib->fcPlcfAtnbkf, pWwFib->lcbPlcfAtnbkf, 4));
-
-    if (mpAtnStarts.get())
-        return mpAtnStarts->GetPos(nIndex);
-    else
-        return SAL_MAX_INT32;
-}
-
-WW8_CP SwWW8ImplReader::GetAnnotationEnd(int nIndex)
-{
-    if (!mpAtnEnds.get() && pWwFib->lcbPlcfAtnbkl)
-        // The Plcfbkl structure is a PLC that contains only CPs and no additional data.
-        mpAtnEnds.reset(new WW8PLCFspecial(pTableStream, pWwFib->fcPlcfAtnbkl, pWwFib->lcbPlcfAtnbkl, 0));
-
-    if (mpAtnEnds.get())
-        return mpAtnEnds->GetPos(nIndex);
-    else
-        return SAL_MAX_INT32;
-}
-
 sal_uLong SwWW8ImplReader::LoadDoc( SwPaM& rPaM,WW8Glossary *pGloss)
 {
     sal_uLong nErrRet = 0;
diff --git a/sw/source/filter/ww8/ww8par.hxx b/sw/source/filter/ww8/ww8par.hxx
index 74842144697b..b3f5c169f10d 100644
--- a/sw/source/filter/ww8/ww8par.hxx
+++ b/sw/source/filter/ww8/ww8par.hxx
@@ -1670,11 +1670,6 @@ private:
 // spaeter zu ersetzen durch Aufruf in entsprechend erweiterten SvxMSDffManager
 
     const OUString* GetAnnotationAuthor(sal_uInt16 nIdx);
-    int GetAnnotationIndex(sal_uInt32 nTag);
-    /// Return the end index based on the start one.
-    sal_uInt16 GetAnnotationEndIndex(sal_uInt16 nStart);
-    WW8_CP GetAnnotationStart(int nIndex);
-    WW8_CP GetAnnotationEnd(int nIndex);
 
     // Schnittstellen fuer die Toggle-Attribute
     void SetToggleAttr(sal_uInt8 nAttrId, bool bOn);
@@ -1736,6 +1731,7 @@ public:     // eigentlich private, geht aber leider nur public
     sal_uInt16 End_Field();
     long Read_Book(WW8PLCFManResult*);
     long Read_And(WW8PLCFManResult* pRes);
+    long Read_AtnBook(WW8PLCFManResult*);
 
     // Attribute
 
diff --git a/sw/source/filter/ww8/ww8par5.cxx b/sw/source/filter/ww8/ww8par5.cxx
index 2d85b8ef8383..d837c8ecfae1 100644
--- a/sw/source/filter/ww8/ww8par5.cxx
+++ b/sw/source/filter/ww8/ww8par5.cxx
@@ -220,6 +220,18 @@ long SwWW8ImplReader::Read_Book(WW8PLCFManResult*)
     return 0;
 }
 
+long SwWW8ImplReader::Read_AtnBook(WW8PLCFManResult*)
+{
+    if (WW8PLCFx_AtnBook* pAtnBook = pPlcxMan->GetAtnBook())
+    {
+        if (pAtnBook->getIsEnd())
+            pReffedStck->SetAttr(*pPaM->GetPoint(), RES_FLTR_ANNOTATIONMARK, true, pAtnBook->getHandle());
+        else
+            pReffedStck->NewAttr(*pPaM->GetPoint(), CntUInt16Item(RES_FLTR_ANNOTATIONMARK, pAtnBook->getHandle()));
+    }
+    return 0;
+}
+
 //    general help methods to separate parameters
 
 /// translate FieldParameter names into the system character set and
diff --git a/sw/source/filter/ww8/ww8par6.cxx b/sw/source/filter/ww8/ww8par6.cxx
index 9310ff9d3c6f..bc8de99986b3 100644
--- a/sw/source/filter/ww8/ww8par6.cxx
+++ b/sw/source/filter/ww8/ww8par6.cxx
@@ -4987,7 +4987,8 @@ long SwWW8ImplReader::ImportExtSprm(WW8PLCFManResult* pRes)
         /* 1 (257) */   &SwWW8ImplReader::Read_Ftn,     // EndNote
         /* 2 (258) */   &SwWW8ImplReader::Read_Field,  // Feld
         /* 3 (259) */   &SwWW8ImplReader::Read_Book,   // Bookmark
-        /* 4 (260) */   &SwWW8ImplReader::Read_And     // Annotation
+        /* 4 (260) */   &SwWW8ImplReader::Read_And,     // Annotation
+        /* 5 (261) */   &SwWW8ImplReader::Read_AtnBook // Annotationmark
     };
 
     if( pRes->nSprmId < 280 )
diff --git a/sw/source/filter/ww8/ww8struc.hxx b/sw/source/filter/ww8/ww8struc.hxx
index e1c8dfa6a979..56d06272d737 100644
--- a/sw/source/filter/ww8/ww8struc.hxx
+++ b/sw/source/filter/ww8/ww8struc.hxx
@@ -981,14 +981,6 @@ struct WW8_STRINGID
     SVBT16 reserved3;
 };
 
-/// The ATNBE structure contains information about an annotation bookmark in the document.
-struct WW8_ATNBE
-{
-    SVBT16 nBmc;
-    SVBT32 nTag;
-    SVBT32 nTagOld;
-};
-
 struct WW8_WKB
 {
     // M.M. This is the WkbPLCF struct