[Libreoffice-commits] core.git: sw/qa sw/source

Miklos Vajna vmiklos at collabora.co.uk
Thu Dec 21 12:31:07 UTC 2017


 sw/qa/python/text_portion_enumeration_test.py |   21 +++-
 sw/source/core/unocore/unoportenum.cxx        |  111 ++++++++++++++++++++++----
 2 files changed, 112 insertions(+), 20 deletions(-)

New commits:
commit 76a4305d1e90b6617054dd33036e64f005dbcf04
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Thu Dec 21 09:32:13 2017 +0100

    sw: fix inconsistent bookmark behavior around at-char/as-char anchored frames
    
    We have a placeholder character in the sw doc model for as-char anchored
    frames, so it's possible to have a bookmark before/after the frame or a
    non-collapsed bookmark which covers the frame.
    
    The same is not true for at-char anchored frames, where the anchor
    points to a doc model position, but there is no placeholder character.
    If a bookmark is created covering the start and end of the anchor of the
    frame, internally we create a collapsed bookmark which has the same
    position as the anchor of the frame. When this doc model is handled by
    SwXParagraph::createEnumeration(), first the frame and then the bookmark
    is appended to the text portion enumeration, so your bookmark around the
    frame is turned into a collapsed bookmark after the frame. (The same
    happens when we roundtrip an ODT document representing this doc model.)
    
    Fix the problem by inserting collapsed bookmarks with affected anchor
    positions (same position is the anchor for an at-char frame) into the
    enumeration in two stages: first the start of them before frames and
    then the end of them + other bookmarks. This way UNO API users get their
    non-collapsed bookmarks around at-char anchored frames, similar to
    as-char ones.
    
    Change-Id: Ic1f173c85d3824afabb5b7ebf3a8594311eb9007
    Reviewed-on: https://gerrit.libreoffice.org/46889
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
    Tested-by: Jenkins <ci at libreoffice.org>

diff --git a/sw/qa/python/text_portion_enumeration_test.py b/sw/qa/python/text_portion_enumeration_test.py
index d6774629f96a..f5a9a56550d9 100644
--- a/sw/qa/python/text_portion_enumeration_test.py
+++ b/sw/qa/python/text_portion_enumeration_test.py
@@ -1429,8 +1429,9 @@ class TextPortionEnumerationTest(unittest.TestCase):
         name5 = self.mkname("frame")
         root = TreeNode()
         root.appendchild(TextNode("abc"))
-        root.appendchild(BookmarkNode(name1))
+        root.appendchild(BookmarkStartNode(name1))
         root.appendchild(FrameNode(name2, AT_CHARACTER))
+        root.appendchild(BookmarkEndNode(name1))
         root.appendchild(ReferenceMarkNode(name3))
         root.appendchild(FrameNode(name4, AT_CHARACTER))
         root.appendchild(FrameNode(name5, AT_CHARACTER))
@@ -1479,12 +1480,14 @@ class TextPortionEnumerationTest(unittest.TestCase):
         root = TreeNode()
         root.appendchild(ReferenceMarkNode(name1))
         root.appendchild(DocumentIndexMarkNode(name2))
-        root.appendchild(BookmarkNode(name3))
+        root.appendchild(BookmarkStartNode(name3))
         root.appendchild(FrameNode(name4, AT_CHARACTER))
+        root.appendchild(BookmarkEndNode(name3))
         root.appendchild(ReferenceMarkNode(name7))
         root.appendchild(DocumentIndexMarkNode(name8))
-        root.appendchild(BookmarkNode(name9))
+        root.appendchild(BookmarkStartNode(name9))
         root.appendchild(FrameNode(nameA, AT_CHARACTER))
+        root.appendchild(BookmarkEndNode(name9))
         self.dotest(root)
 
     def test_empty2(self):
@@ -1493,10 +1496,12 @@ class TextPortionEnumerationTest(unittest.TestCase):
         name9 = self.mkname("bookmark")
         nameA = self.mkname("frame")
         root = TreeNode()
-        root.appendchild(BookmarkNode(name3))
+        root.appendchild(BookmarkStartNode(name3))
         root.appendchild(FrameNode(name4, AT_CHARACTER))
-        root.appendchild(BookmarkNode(name9))
+        root.appendchild(BookmarkEndNode(name3))
+        root.appendchild(BookmarkStartNode(name9))
         root.appendchild(FrameNode(nameA, AT_CHARACTER))
+        root.appendchild(BookmarkEndNode(name9))
         self.dotest(root)
 
     def test_empty3(self):
@@ -1513,8 +1518,9 @@ class TextPortionEnumerationTest(unittest.TestCase):
         root = TreeNode()
         root.appendchild(ReferenceMarkNode(name1))
         root.appendchild(DocumentIndexMarkNode(name2))
-        root.appendchild(BookmarkNode(name3))
+        root.appendchild(BookmarkStartNode(name3))
         root.appendchild(FrameNode(name4, AT_CHARACTER))
+        root.appendchild(BookmarkEndNode(name3))
         ## currently empty hyperlinks may get eaten...
         # href = HyperlinkNode(name5)
         # href.appendchild(TextNode(""))
@@ -1523,8 +1529,9 @@ class TextPortionEnumerationTest(unittest.TestCase):
         root.appendchild(ruby)
         root.appendchild(ReferenceMarkNode(name7))
         root.appendchild(DocumentIndexMarkNode(name8))
-        root.appendchild(BookmarkNode(name9))
+        root.appendchild(BookmarkStartNode(name9))
         root.appendchild(FrameNode(nameA, AT_CHARACTER))
+        root.appendchild(BookmarkEndNode(name9))
         self.dotest(root)
 
     def test1(self):
diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx
index d40ec5bf5135..50178a3c07a7 100644
--- a/sw/source/core/unocore/unoportenum.cxx
+++ b/sw/source/core/unocore/unoportenum.cxx
@@ -592,43 +592,77 @@ lcl_CreateMetaPortion(
     return pPortion;
 }
 
+/**
+ * Exports all bookmarks from rBkmArr into rPortions that have the same start
+ * or end position as nIndex.
+ *
+ * @param rBkmArr the array of bookmarks. If bOnlyFrameStarts is true, then
+ * this is only read, otherwise consumed entries are removed.
+ *
+ * @param rFramePositions the list of positions where there is an at-char /
+ * anchored frame.
+ *
+ * @param bOnlyFrameStarts If true: export only the start of the bookmarks
+ * which cover an at-char anchored frame. If false: export the end of the same
+ * bookmarks and everything else.
+ */
 static void lcl_ExportBookmark(
     TextRangeList_t & rPortions,
     Reference<XText> const& xParent,
     const SwUnoCursor * const pUnoCursor,
     SwXBookmarkPortion_ImplList& rBkmArr,
-    const sal_Int32 nIndex)
+    const sal_Int32 nIndex,
+    const std::set<sal_Int32>& rFramePositions,
+    bool bOnlyFrameStarts)
 {
     for ( SwXBookmarkPortion_ImplList::iterator aIter = rBkmArr.begin(), aEnd = rBkmArr.end(); aIter != aEnd; )
     {
         SwXBookmarkPortion_ImplSharedPtr pPtr = (*aIter);
         if ( nIndex > pPtr->getIndex() )
         {
-            aIter = rBkmArr.erase(aIter);
+            if (bOnlyFrameStarts)
+                ++aIter;
+            else
+                aIter = rBkmArr.erase(aIter);
             continue;
         }
         if ( nIndex < pPtr->getIndex() )
             break;
 
         SwXTextPortion* pPortion = nullptr;
-        if ((BkmType::Start     == pPtr->nBkmType) ||
+        if ((BkmType::Start == pPtr->nBkmType && !bOnlyFrameStarts) ||
             (BkmType::StartEnd == pPtr->nBkmType))
         {
-            pPortion =
-                new SwXTextPortion(pUnoCursor, xParent, PORTION_BOOKMARK_START);
-            rPortions.emplace_back(pPortion);
-            pPortion->SetBookmark(pPtr->xBookmark);
-            pPortion->SetCollapsed( BkmType::StartEnd == pPtr->nBkmType );
+            bool bFrameStart = rFramePositions.find(nIndex) != rFramePositions.end();
+            bool bEnd = pPtr->nBkmType == BkmType::StartEnd && bFrameStart && !bOnlyFrameStarts;
+            if (pPtr->nBkmType == BkmType::Start || bFrameStart || !bOnlyFrameStarts)
+            {
+                // At this we create a text portion, due to one of these
+                // reasons:
+                // - this is the real start of a non-collapsed bookmark
+                // - this is the real position of a collapsed bookmark
+                // - this is the start or end (depending on bOnlyFrameStarts)
+                //   of a collapsed bookmark at the same position as an at-char
+                //   anchored frame
+                pPortion =
+                    new SwXTextPortion(pUnoCursor, xParent, bEnd ? PORTION_BOOKMARK_END : PORTION_BOOKMARK_START);
+                rPortions.emplace_back(pPortion);
+                pPortion->SetBookmark(pPtr->xBookmark);
+                pPortion->SetCollapsed( BkmType::StartEnd == pPtr->nBkmType && !bFrameStart );
+            }
 
         }
-        if (BkmType::End == pPtr->nBkmType)
+        if (BkmType::End == pPtr->nBkmType && !bOnlyFrameStarts)
         {
             pPortion =
                 new SwXTextPortion(pUnoCursor, xParent, PORTION_BOOKMARK_END);
             rPortions.emplace_back(pPortion);
             pPortion->SetBookmark(pPtr->xBookmark);
         }
-        aIter = rBkmArr.erase(aIter);
+        if (bOnlyFrameStarts)
+            ++aIter;
+        else
+            aIter = rBkmArr.erase(aIter);
     }
 }
 
@@ -1117,10 +1151,18 @@ static void lcl_ExportBkmAndRedline(
     SwXBookmarkPortion_ImplList& rBkmArr,
     SwXRedlinePortion_ImplList& rRedlineArr,
     SwSoftPageBreakList& rBreakArr,
-    const sal_Int32 nIndex)
+    const sal_Int32 nIndex,
+    const std::set<sal_Int32>& rFramePositions,
+    bool bOnlyFrameBookmarkStarts)
 {
     if (!rBkmArr.empty())
-        lcl_ExportBookmark(rPortions, xParent, pUnoCursor, rBkmArr, nIndex);
+        lcl_ExportBookmark(rPortions, xParent, pUnoCursor, rBkmArr, nIndex, rFramePositions,
+                           bOnlyFrameBookmarkStarts);
+
+    if (bOnlyFrameBookmarkStarts)
+        // Only exporting the start of some collapsed bookmarks: no export of
+        // other arrays.
+        return;
 
     if (!rRedlineArr.empty())
         lcl_ExportRedline(rPortions, xParent, pUnoCursor, rRedlineArr, nIndex);
@@ -1162,6 +1204,38 @@ static void lcl_ExportAnnotationStarts(
     }
 }
 
+/// Fills character positions from rFrames into rFramePositions.
+static void lcl_ExtractFramePositions(FrameClientSortList_t& rFrames, sal_Int32 nCurrentIndex,
+                                      std::set<sal_Int32>& rFramePositions)
+{
+    for (const auto& rFrame : rFrames)
+    {
+        if (rFrame.nIndex < nCurrentIndex)
+            continue;
+
+        if (rFrame.nIndex > nCurrentIndex)
+            break;
+
+        const SwModify* pFrame = rFrame.pFrameClient->GetRegisteredIn();
+        if (!pFrame)
+            continue;
+
+        auto& rFormat = *static_cast<SwFrameFormat*>(const_cast<SwModify*>(pFrame));
+        const SwFormatAnchor& rAnchor = rFormat.GetAnchor();
+        const SwPosition* pPosition = rAnchor.GetContentAnchor();
+        if (!pPosition)
+            continue;
+
+        rFramePositions.insert(pPosition->nContent.GetIndex());
+    }
+}
+
+/**
+ * Exports at-char anchored frames.
+ *
+ * @param i_rFrames the frames for this paragraph, frames at <= i_nCurrentIndex
+ * are removed from the container.
+ */
 static sal_Int32 lcl_ExportFrames(
     TextRangeList_t & rPortions,
     Reference<XText> const & i_xParent,
@@ -1286,12 +1360,23 @@ static void lcl_CreatePortions(
 
         SwUnoCursorHelper::SelectPam(*pUnoCursor, true); // set mark
 
+        // First remember the frame positions.
+        std::set<sal_Int32> aFramePositions;
+        lcl_ExtractFramePositions(i_rFrames, nCurrentIndex, aFramePositions);
+
+        // Then export start of collapsed bookmarks which "cover" at-char
+        // anchored frames.
+        lcl_ExportBkmAndRedline( *PortionStack.top().first, i_xParentText,
+            pUnoCursor, Bookmarks, Redlines, SoftPageBreaks, nCurrentIndex, aFramePositions, /*bOnlyFrameBookmarkStarts=*/true );
+
         const sal_Int32 nFirstFrameIndex =
             lcl_ExportFrames( *PortionStack.top().first,
                 i_xParentText, pUnoCursor, i_rFrames, nCurrentIndex);
 
+        // Export ends of the previously started collapsed bookmarks + all
+        // other bookmarks, redlines, etc.
         lcl_ExportBkmAndRedline( *PortionStack.top().first, i_xParentText,
-            pUnoCursor, Bookmarks, Redlines, SoftPageBreaks, nCurrentIndex );
+            pUnoCursor, Bookmarks, Redlines, SoftPageBreaks, nCurrentIndex, aFramePositions, /*bOnlyFrameBookmarkStarts=*/false );
 
         lcl_ExportAnnotationStarts(
             *PortionStack.top().first,


More information about the Libreoffice-commits mailing list