From ff536c6c479a160932e316e5099848151091b63c Mon Sep 17 00:00:00 2001 From: Serge Krot Date: Fri, 24 Aug 2018 13:31:54 +0200 Subject: sw: fix inconsistent bookmark behavior around at-char/as-char anchored frames Added fix for Change-Id: Ic1f173c85d3824afabb5b7ebf3a8594311eb9007 Reviewed-on: https://gerrit.libreoffice.org/46889 Reviewed-by: Miklos Vajna Tested-by: Jenkins The problem was (the same condition of the bOnlyFrameStarts parameter was used during output of Start and End bookmarks): if (BkmType::Start == pPtr->nBkmType && !bOnlyFrameStarts) ... if (BkmType::End == pPtr->nBkmType && !bOnlyFrameStarts) ... Should be: if (BkmType::Start == pPtr->nBkmType && bOnlyFrameStarts) ... if (BkmType::End == pPtr->nBkmType && !bOnlyFrameStarts) ... I assume this was a simple copy-paste bug. Reviewed-on: https://gerrit.libreoffice.org/59556 Tested-by: Jenkins Reviewed-by: Thorsten Behrens Conflicts: sw/source/core/unocore/unoportenum.cxx It looks like you may be committing a cherry-pick. If this is not correct, please remove the file .git/CHERRY_PICK_HEAD and try again. sw: fix inconsistent bookmark behavior around at-char/as-char anchored frames Added unit test for Added fix for Change-Id: Ic1f173c85d3824afabb5b7ebf3a8594311eb9007 Reviewed-on: https://gerrit.libreoffice.org/59828 Tested-by: Jenkins Reviewed-by: Thorsten Behrens Conflicts: sw/qa/extras/uiwriter/uiwriter.cxx Changes to be committed: new file: sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott modified: sw/qa/extras/uiwriter/uiwriter.cxx 38444587d00b96d52ff725dc7c5852e057bc6bd9 Reviewed-on: https://gerrit.libreoffice.org/59854 Reviewed-by: Thorsten Behrens Tested-by: Thorsten Behrens Change-Id: I712a0dccc1638fed3b81c65628033a4dc06c1ca4 --- .../uiwriter/data/testInconsistentBookmark.ott | Bin 0 -> 10488 bytes sw/qa/extras/uiwriter/uiwriter.cxx | 61 +++++++++++++++++++++ sw/source/core/unocore/unoportenum.cxx | 14 ++--- 3 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott diff --git a/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott b/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott new file mode 100644 index 000000000000..ff3970a27b27 Binary files /dev/null and b/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott differ diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx index 2410ab136a8e..99e7bc5b20f7 100755 --- a/sw/qa/extras/uiwriter/uiwriter.cxx +++ b/sw/qa/extras/uiwriter/uiwriter.cxx @@ -217,6 +217,7 @@ public: void testTdf113877NoMerge(); void testTdf113877_default_style(); void testTdf113877_Standard_style(); + void testInconsistentBookmark(); CPPUNIT_TEST_SUITE(SwUiWriterTest); CPPUNIT_TEST(testReplaceForward); @@ -332,6 +333,7 @@ public: CPPUNIT_TEST(testTdf113877NoMerge); CPPUNIT_TEST(testTdf113877_default_style); CPPUNIT_TEST(testTdf113877_Standard_style); + CPPUNIT_TEST(testInconsistentBookmark); CPPUNIT_TEST_SUITE_END(); private: @@ -4021,6 +4023,65 @@ void SwUiWriterTest::testTdf113877_Standard_style() CPPUNIT_ASSERT_EQUAL(listId1, listId3); } +// Unit test for 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. +void SwUiWriterTest::testInconsistentBookmark() +{ + // create test document with text and bookmark + { + SwDoc* pDoc(createDoc("testInconsistentBookmark.ott")); + IDocumentMarkAccess& rIDMA(*pDoc->getIDocumentMarkAccess()); + SwNodeIndex aIdx(pDoc->GetNodes().GetEndOfContent(), -1); + SwCursor aPaM(SwPosition(aIdx), nullptr); + aPaM.SetMark(); + aPaM.MovePara(GetfnParaCurr(), GetfnParaStart()); + aPaM.MovePara(GetfnParaCurr(), GetfnParaEnd()); + rIDMA.makeMark(aPaM, "Mark", IDocumentMarkAccess::MarkType::BOOKMARK); + aPaM.Exchange(); + aPaM.DeleteMark(); + } + + // save document and verify the bookmark scoup + { + // save document + utl::TempFile aTempFile; + save("writer8", aTempFile); + + // load only content.xml + if (xmlDocPtr pXmlDoc = parseExportInternal(aTempFile.GetURL(), "content.xml")) + { + const OString aPath("/office:document-content/office:body/office:text/text:p"); + + const OUString aTagBookmarkStart("bookmark-start"); + const OUString aTagControl("control"); + const OUString aTagBookmarkEnd("bookmark-end"); + + const int pos1 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkStart); + const int pos2 = getXPathPosition(pXmlDoc, aPath, aTagControl); + const int pos3 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkEnd); + + CPPUNIT_ASSERT(pos1 < pos2); + CPPUNIT_ASSERT(pos2 < pos3); + } + } +} + CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx index 384d4cc6ed7a..601c53305c04 100644 --- a/sw/source/core/unocore/unoportenum.cxx +++ b/sw/source/core/unocore/unoportenum.cxx @@ -627,7 +627,7 @@ static void lcl_ExportBookmark( { for ( SwXBookmarkPortion_ImplList::iterator aIter = rBkmArr.begin(), aEnd = rBkmArr.end(); aIter != aEnd; ) { - SwXBookmarkPortion_ImplSharedPtr pPtr = (*aIter); + const SwXBookmarkPortion_ImplSharedPtr& pPtr = (*aIter); if ( nIndex > pPtr->getIndex() ) { if (bOnlyFrameStarts) @@ -639,8 +639,7 @@ static void lcl_ExportBookmark( if ( nIndex < pPtr->getIndex() ) break; - SwXTextPortion* pPortion = nullptr; - if ((BKM_TYPE_START == pPtr->nBkmType && !bOnlyFrameStarts) || + if ((BKM_TYPE_START == pPtr->nBkmType && bOnlyFrameStarts) || (BKM_TYPE_START_END == pPtr->nBkmType)) { bool bFrameStart = rFramePositions.find(nIndex) != rFramePositions.end(); @@ -654,21 +653,22 @@ static void lcl_ExportBookmark( // - this is the start or end (depending on bOnlyFrameStarts) // of a collapsed bookmark at the same position as an at-char // anchored frame - pPortion = + SwXTextPortion* pPortion = new SwXTextPortion(pUnoCursor, xParent, bEnd ? PORTION_BOOKMARK_END : PORTION_BOOKMARK_START); rPortions.emplace_back(pPortion); pPortion->SetBookmark(pPtr->xBookmark); pPortion->SetCollapsed( BKM_TYPE_START_END == pPtr->nBkmType && !bFrameStart ); } - } - if (BKM_TYPE_END == pPtr->nBkmType && !bOnlyFrameStarts) + else if (BKM_TYPE_END == pPtr->nBkmType && !bOnlyFrameStarts) { - pPortion = + SwXTextPortion* pPortion = new SwXTextPortion(pUnoCursor, xParent, PORTION_BOOKMARK_END); rPortions.push_back(pPortion); pPortion->SetBookmark(pPtr->xBookmark); } + + // next bookmark if (bOnlyFrameStarts) ++aIter; else -- cgit