diff options
author | Vasily Melenchuk <vasily.melenchuk@cib.de> | 2021-09-09 11:41:33 +0300 |
---|---|---|
committer | Thorsten Behrens <thorsten.behrens@allotropia.de> | 2021-09-14 10:00:24 +0200 |
commit | 683842708664e37210eb0db5f974c042b1ee7ff6 (patch) | |
tree | e42d12a217c785dc689401cb19140114a6247ef0 | |
parent | tdf#144106 Don't proceed ptrim_i past ptrim_f (diff) | |
download | core-683842708664e37210eb0db5f974c042b1ee7ff6.tar.gz core-683842708664e37210eb0db5f974c042b1ee7ff6.zip |
tdf#123642: keep last bookmark at the document end
In some cases DomainMapper_Impl::RemoveLastParagraph() can also remove
last bookmark from real last paragraph. This does never happens
when we use xParagraph->dispose(), but pretty always during older
way with xCursor->setString(OUString()).
Unfortunately without deep refactoring of redlines, bookmarks, etc.
I see no other way to avoid this removal except given hack which is
trying to store last bookmark and if it did disappear restore it.
Some existing unittests were adjusted: corresponding original DOCX
files did contain final bookmarks not taken into account by the code.
In some cases test code is modified, in some just removed final
bookmark in DOCX: such multi-format tests as in ooxmlfieldexport.cxx
will be more identical to RTF & DOC variants.
Conflicts:
sw/qa/extras/ooxmlexport/ooxmlexport17.cxx
Change-Id: Ie9948b58cda705a0b85fa8e5e08b72fbb7d682b2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121409
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <thorsten.behrens@allotropia.de>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122015
-rw-r--r-- | sw/qa/extras/globalfilter/data/char_background_editing.docx | bin | 12713 -> 12672 bytes | |||
-rw-r--r-- | sw/qa/extras/globalfilter/data/fieldmark_QUOTE_nest.docx | bin | 13183 -> 13179 bytes | |||
-rw-r--r-- | sw/qa/extras/layout/layout.cxx | 2 | ||||
-rw-r--r-- | sw/qa/extras/ooxmlexport/data/tdf123642.docx | bin | 0 -> 17814 bytes | |||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlexport17.cxx | 21 | ||||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlexport8.cxx | 2 | ||||
-rw-r--r-- | sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx | 6 | ||||
-rw-r--r-- | writerfilter/source/dmapper/DomainMapper_Impl.cxx | 75 |
8 files changed, 100 insertions, 6 deletions
diff --git a/sw/qa/extras/globalfilter/data/char_background_editing.docx b/sw/qa/extras/globalfilter/data/char_background_editing.docx Binary files differindex 3e3302d00c71..5e92fe8bbd49 100644 --- a/sw/qa/extras/globalfilter/data/char_background_editing.docx +++ b/sw/qa/extras/globalfilter/data/char_background_editing.docx diff --git a/sw/qa/extras/globalfilter/data/fieldmark_QUOTE_nest.docx b/sw/qa/extras/globalfilter/data/fieldmark_QUOTE_nest.docx Binary files differindex ba886edfa777..9fbec01e0ab6 100644 --- a/sw/qa/extras/globalfilter/data/fieldmark_QUOTE_nest.docx +++ b/sw/qa/extras/globalfilter/data/fieldmark_QUOTE_nest.docx diff --git a/sw/qa/extras/layout/layout.cxx b/sw/qa/extras/layout/layout.cxx index 12fb7c421967..c13333d3a86b 100644 --- a/sw/qa/extras/layout/layout.cxx +++ b/sw/qa/extras/layout/layout.cxx @@ -1319,7 +1319,7 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf116486) { SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf116486.docx"); CPPUNIT_ASSERT(pDoc); - OUString aTop = parseDump("/root/page/body/txt/Special", "nHeight"); + OUString aTop = parseDump("/root/page/body/txt/Special[1]", "nHeight"); CPPUNIT_ASSERT_EQUAL(OUString("4006"), aTop); } diff --git a/sw/qa/extras/ooxmlexport/data/tdf123642.docx b/sw/qa/extras/ooxmlexport/data/tdf123642.docx Binary files differnew file mode 100644 index 000000000000..9817093e02f8 --- /dev/null +++ b/sw/qa/extras/ooxmlexport/data/tdf123642.docx diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx index 4eb9d0d6aa1b..6884ea3ebc03 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport17.cxx @@ -10,6 +10,7 @@ #include <sal/config.h> #include <string_view> +#include <com/sun/star/text/XBookmarksSupplier.hpp> #include <swmodeltestbase.hxx> @@ -42,6 +43,26 @@ DECLARE_OOXMLEXPORT_TEST(testTdf135164_cancelledNumbering, "tdf135164_cancelledN CPPUNIT_ASSERT_EQUAL(OUString("i"), getProperty<OUString>(xPara, "ListLabelString")); } +DECLARE_OOXMLEXPORT_TEST(testTdf123642_BookmarkAtDocEnd, "tdf123642.docx") +{ + // get bookmark interface + uno::Reference<text::XBookmarksSupplier> xBookmarksSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xBookmarksByIdx(xBookmarksSupplier->getBookmarks(), uno::UNO_QUERY); + uno::Reference<container::XNameAccess> xBookmarksByName = xBookmarksSupplier->getBookmarks(); + + // check: we have 1 bookmark (previously there were 0) + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), xBookmarksByIdx->getCount()); + CPPUNIT_ASSERT(xBookmarksByName->hasByName("Bookmark1")); + + // and it is really in exprted DOCX (let's ensure) + xmlDocUniquePtr pXmlDoc = parseExport("word/document.xml"); + if (!pXmlDoc) + return; // initial import, no futher checks + + CPPUNIT_ASSERT_EQUAL(OUString("Bookmark1"), getXPath(pXmlDoc, "/w:document/w:body/w:p[2]/w:bookmarkStart[1]", "name")); +} + + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx index 0faed8c2ac02..049b9091b81a 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx @@ -1036,7 +1036,7 @@ DECLARE_OOXMLEXPORT_TEST(testN820509, "n820509.docx") CPPUNIT_ASSERT(pTextDoc); SwDoc* pDoc = pTextDoc->GetDocShell()->GetDoc(); IDocumentMarkAccess* pMarkAccess = pDoc->getIDocumentMarkAccess(); - CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMarkAccess->getAllMarksCount()); ::sw::mark::IFieldmark* pFieldmark = dynamic_cast<::sw::mark::IFieldmark*>(*pMarkAccess->getAllMarksBegin()); diff --git a/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx b/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx index d92c29fefbe7..f6e6ea78bc99 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlfieldexport.cxx @@ -455,7 +455,7 @@ DECLARE_OOXMLEXPORT_TEST(testSdtDateDuplicate, "sdt-date-duplicate.docx") CPPUNIT_ASSERT(pTextDoc); SwDoc* pDoc = pTextDoc->GetDocShell()->GetDoc(); IDocumentMarkAccess* pMarkAccess = pDoc->getIDocumentMarkAccess(); - CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMarkAccess->getAllMarksCount()); ::sw::mark::IDateFieldmark* pFieldmark = dynamic_cast<::sw::mark::IDateFieldmark*>(*pMarkAccess->getAllMarksBegin()); @@ -614,7 +614,7 @@ DECLARE_OOXMLEXPORT_TEST( testDateFieldInShape, "date_field_in_shape.docx" ) CPPUNIT_ASSERT(pTextDoc); SwDoc* pDoc = pTextDoc->GetDocShell()->GetDoc(); IDocumentMarkAccess* pMarkAccess = pDoc->getIDocumentMarkAccess(); - CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMarkAccess->getAllMarksCount()); ::sw::mark::IDateFieldmark* pFieldmark = dynamic_cast<::sw::mark::IDateFieldmark*>(*pMarkAccess->getAllMarksBegin()); @@ -630,7 +630,7 @@ DECLARE_OOXMLEXPORT_TEST( testDateFieldAtEndOfParagraph, "date_field_at_end_of_p CPPUNIT_ASSERT(pTextDoc); SwDoc* pDoc = pTextDoc->GetDocShell()->GetDoc(); IDocumentMarkAccess* pMarkAccess = pDoc->getIDocumentMarkAccess(); - CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMarkAccess->getAllMarksCount()); ::sw::mark::IDateFieldmark* pFieldmark = dynamic_cast<::sw::mark::IDateFieldmark*>(*pMarkAccess->getAllMarksBegin()); diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index f4a1f81cbdc3..6202d7b92790 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -564,6 +564,52 @@ void DomainMapper_Impl::AddDummyParaForTableInSection() } } + static OUString lcl_FindLastBookmark(const uno::Reference<text::XTextCursor>& xCursor) + { + OUString sName; + if (!xCursor.is()) + return sName; + + // Select 1 previous element + xCursor->goLeft(1, true); + uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xCursor, uno::UNO_QUERY); + if (!xParaEnumAccess.is()) + { + xCursor->goRight(1, true); + return sName; + } + + // Iterate through selection paragraphs + uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration(); + if (!xParaEnum->hasMoreElements()) + { + xCursor->goRight(1, true); + return sName; + } + + // Iterate through first para portions + uno::Reference<container::XEnumerationAccess> xRunEnumAccess(xParaEnum->nextElement(), + uno::UNO_QUERY_THROW); + uno::Reference<container::XEnumeration> xRunEnum = xRunEnumAccess->createEnumeration(); + while (xRunEnum->hasMoreElements()) + { + uno::Reference<beans::XPropertySet> xProps(xRunEnum->nextElement(), uno::UNO_QUERY_THROW); + uno::Any aType(xProps->getPropertyValue("TextPortionType")); + OUString sType; + aType >>= sType; + if (sType == "Bookmark") + { + uno::Reference<container::XNamed> xBookmark(xProps->getPropertyValue("Bookmark"), + uno::UNO_QUERY_THROW); + sName = xBookmark->getName(); + // Do not stop the scan here. Maybe there are 2 bookmarks? + } + } + + xCursor->goRight(1, true); + return sName; + } + void DomainMapper_Impl::RemoveLastParagraph( ) { if (m_bDiscardHeaderFooter) @@ -593,6 +639,13 @@ void DomainMapper_Impl::RemoveLastParagraph( ) // (but only for paste/insert, not load; otherwise it can happen that // flys anchored at the disposed paragraph are deleted (fdo47036.rtf)) bool const bEndOfDocument(m_aTextAppendStack.size() == 1); + + // Try to find and remember last bookmark in document: it potentially + // can be deleted by xCursor->setString() but not by xParagraph->dispose() + OUString sLastBookmarkName; + if (bEndOfDocument) + sLastBookmarkName = lcl_FindLastBookmark(xCursor); + if ((IsInHeaderFooter() || (bEndOfDocument && !m_bIsNewDoc)) && xEnumerationAccess.is()) { @@ -620,7 +673,27 @@ void DomainMapper_Impl::RemoveLastParagraph( ) // delete xCursor->setString(OUString()); - // restore again + // call to xCursor->setString possibly did remove final bookmark + // from previous paragraph. We need to restore it, if there was any. + if (sLastBookmarkName.getLength()) + { + OUString sBookmarkNameAfterRemoval = lcl_FindLastBookmark(xCursor); + if (sBookmarkNameAfterRemoval.isEmpty()) + { + // Yes, it was removed. Restore + uno::Reference<text::XTextContent> xBookmark( + m_xTextFactory->createInstance("com.sun.star.text.Bookmark"), + uno::UNO_QUERY_THROW); + + uno::Reference<container::XNamed> xBkmNamed(xBookmark, + uno::UNO_QUERY_THROW); + xBkmNamed->setName(sLastBookmarkName); + xTextAppend->insertTextContent( + uno::Reference<text::XTextRange>(xCursor, uno::UNO_QUERY_THROW), + xBookmark, !xCursor->isCollapsed()); + } + } + // restore redline options again xDocProps->setPropertyValue(aRecordChanges, aPreviousValue); } } |