diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2020-10-12 21:04:36 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2020-10-13 09:02:17 +0200 |
commit | 3ee3ae85de3a29ebfb89e75960b65417bfd6ca55 (patch) | |
tree | d8bcc80544393216c1b6da6a2d8d07465de8df85 /writerfilter | |
parent | 4f8b13267559e8e89a306a6c522f905322396bd0 (diff) |
tdf#137180 RTF import: fix bad left margin with direct format/para style/num
This is a case when a left margin appears as direct formatting on a
paragraph, the paragraph has a style and the style has the same left
margin. But the paragraph has a numbering as well: so it's not valid to
deduplicate the left margin from the direct formatting, because then the
left margin from the numbering will be used, which can be a different
value.
Change-Id: I5e27502b8d505bdfddfdc910858f62e501db35a5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104220
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Diffstat (limited to 'writerfilter')
-rw-r--r-- | writerfilter/CppunitTest_writerfilter_rtftok.mk | 1 | ||||
-rw-r--r-- | writerfilter/qa/cppunittests/rtftok/data/left-margin-dedup.rtf | 26 | ||||
-rw-r--r-- | writerfilter/qa/cppunittests/rtftok/rtfsprm.cxx | 83 | ||||
-rw-r--r-- | writerfilter/source/rtftok/rtfdocumentimpl.cxx | 2 | ||||
-rw-r--r-- | writerfilter/source/rtftok/rtfsprm.cxx | 25 | ||||
-rw-r--r-- | writerfilter/source/rtftok/rtfsprm.hxx | 5 |
6 files changed, 130 insertions, 12 deletions
diff --git a/writerfilter/CppunitTest_writerfilter_rtftok.mk b/writerfilter/CppunitTest_writerfilter_rtftok.mk index 4ca730b9d3fc..db038292ebdd 100644 --- a/writerfilter/CppunitTest_writerfilter_rtftok.mk +++ b/writerfilter/CppunitTest_writerfilter_rtftok.mk @@ -17,6 +17,7 @@ $(eval $(call gb_CppunitTest_use_externals,writerfilter_rtftok,\ $(eval $(call gb_CppunitTest_add_exception_objects,writerfilter_rtftok, \ writerfilter/qa/cppunittests/rtftok/rtfsdrimport \ + writerfilter/qa/cppunittests/rtftok/rtfsprm \ )) $(eval $(call gb_CppunitTest_use_libraries,writerfilter_rtftok, \ diff --git a/writerfilter/qa/cppunittests/rtftok/data/left-margin-dedup.rtf b/writerfilter/qa/cppunittests/rtftok/data/left-margin-dedup.rtf new file mode 100644 index 000000000000..8536694a294b --- /dev/null +++ b/writerfilter/qa/cppunittests/rtftok/data/left-margin-dedup.rtf @@ -0,0 +1,26 @@ +{\rtf1 +{\fonttbl +{\f0 Times New Roman;} +{\f3\froman\fcharset2\fprq2 Symbol;} +} +{\stylesheet +{\ql Normal;} +{\s44\ql\fi-288\li360 Table Text Bullet;} +{\s59\ql\li720 List Paragraph;} +} +{\*\listtable +{\list +{\listlevel\levelnfc23\leveljc0\leveljcn0\levelfollow0 +\levelstartat1\levelspace0\levelindent0 +{\leveltext\'01\u-3913 ?;} +{\levelnumbers;} +\f3\fbias0 \fi-360\li1305 } +{\listname ;} +\listid845246918} +} +{\*\listoverridetable +{\listoverride\listid845246918\listoverridecount0\ls27} +} +\pard\plain\s44\ql\fi-360\li720\ls27 P1\par +\pard\plain\s59\ql\fi-360\li720\ls27 P2\par +} diff --git a/writerfilter/qa/cppunittests/rtftok/rtfsprm.cxx b/writerfilter/qa/cppunittests/rtftok/rtfsprm.cxx new file mode 100644 index 000000000000..9f5ec63e8b3f --- /dev/null +++ b/writerfilter/qa/cppunittests/rtftok/rtfsprm.cxx @@ -0,0 +1,83 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <test/bootstrapfixture.hxx> +#include <unotest/macros_test.hxx> + +#include <com/sun/star/beans/XPropertySet.hpp> +#include <com/sun/star/beans/XPropertyState.hpp> +#include <com/sun/star/frame/Desktop.hpp> +#include <com/sun/star/text/XTextDocument.hpp> + +using namespace ::com::sun::star; + +namespace +{ +/// Tests for writerfilter/source/rtftok/rtfsprm.cxx. +class Test : public test::BootstrapFixture, public unotest::MacrosTest +{ +private: + uno::Reference<lang::XComponent> mxComponent; + +public: + void setUp() override; + void tearDown() override; + uno::Reference<lang::XComponent>& getComponent() { return mxComponent; } +}; + +void Test::setUp() +{ + test::BootstrapFixture::setUp(); + + mxDesktop.set(frame::Desktop::create(mxComponentContext)); +} + +void Test::tearDown() +{ + if (mxComponent.is()) + mxComponent->dispose(); + + test::BootstrapFixture::tearDown(); +} + +char const DATA_DIRECTORY[] = "/writerfilter/qa/cppunittests/rtftok/data/"; + +CPPUNIT_TEST_FIXTURE(Test, testLeftMarginDedup) +{ + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "left-margin-dedup.rtf"; + getComponent() = loadFromDesktop(aURL); + uno::Reference<text::XTextDocument> xTextDocument(getComponent(), uno::UNO_QUERY); + uno::Reference<container::XEnumerationAccess> xText(xTextDocument->getText(), uno::UNO_QUERY); + uno::Reference<container::XEnumeration> xParagraphs = xText->createEnumeration(); + uno::Reference<beans::XPropertySet> xParagraph(xParagraphs->nextElement(), uno::UNO_QUERY); + sal_Int32 nLeftMargin = 0; + xParagraph->getPropertyValue("ParaLeftMargin") >>= nLeftMargin; + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1270), nLeftMargin); + + uno::Reference<beans::XPropertyState> xParagraphState(xParagraph, uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(beans::PropertyState_DIRECT_VALUE, + xParagraphState->getPropertyState("ParaLeftMargin")); + + xParagraph.set(xParagraphs->nextElement(), uno::UNO_QUERY); + nLeftMargin = 0; + xParagraph->getPropertyValue("ParaLeftMargin") >>= nLeftMargin; + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1270), nLeftMargin); + + xParagraphState.set(xParagraph, uno::UNO_QUERY); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 0 (DIRECT_VALUE) + // - Actual : 1 (DEFAULT_VALUE) + // i.e. the left margin was not a direct formatting, which means left margin from the numbering + // was used instead. + CPPUNIT_ASSERT_EQUAL(beans::PropertyState_DIRECT_VALUE, + xParagraphState->getPropertyState("ParaLeftMargin")); +} +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx index bbcec8792bc1..02aa834c38ef 100644 --- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx +++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx @@ -525,7 +525,7 @@ RTFDocumentImpl::getProperties(const RTFSprms& rAttributes, RTFSprms const& rSpr } // Get rid of direct formatting what is already in the style. - RTFSprms const sprms(aSprms.cloneAndDeduplicate(aStyleSprms, nStyleType, true)); + RTFSprms const sprms(aSprms.cloneAndDeduplicate(aStyleSprms, nStyleType, true, &aSprms)); RTFSprms const attributes( rAttributes.cloneAndDeduplicate(aStyleAttributes, nStyleType, true)); return new RTFReferenceProperties(attributes, sprms); diff --git a/writerfilter/source/rtftok/rtfsprm.cxx b/writerfilter/source/rtftok/rtfsprm.cxx index d281187277de..8c54fe95e345 100644 --- a/writerfilter/source/rtftok/rtfsprm.cxx +++ b/writerfilter/source/rtftok/rtfsprm.cxx @@ -196,7 +196,7 @@ static RTFValue::Pointer_t getDefaultSPRM(Id const id, Id nStyleType) } /// Is it problematic to deduplicate this SPRM? -static bool isSPRMDeduplicateDenylist(Id nId) +static bool isSPRMDeduplicateDenylist(Id nId, RTFSprms* pDirect) { switch (nId) { @@ -223,6 +223,11 @@ static bool isSPRMDeduplicateDenylist(Id nId) case NS_ooxml::LN_CT_Border_themeTint: case NS_ooxml::LN_CT_Border_themeColor: return true; + // Removing \fi and \li if the style has the same value would mean taking these values from + // \ls, while deduplication would be done to take the values from the style. + case NS_ooxml::LN_CT_Ind_firstLine: + case NS_ooxml::LN_CT_Ind_left: + return pDirect && pDirect->find(NS_ooxml::LN_CT_PPrBase_numPr); default: return false; @@ -252,22 +257,24 @@ static bool isSPRMChildrenExpected(Id nId) /// Does the clone / deduplication of a single sprm. static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t> const& rSprm, RTFSprms& ret, - Id nStyleType) + Id nStyleType, RTFSprms* pDirect = nullptr) { RTFValue::Pointer_t const pValue(ret.find(rSprm.first)); if (pValue) { if (rSprm.second->equals(*pValue)) { - if (!isSPRMDeduplicateDenylist(rSprm.first)) + if (!isSPRMDeduplicateDenylist(rSprm.first, pDirect)) + { ret.erase(rSprm.first); // duplicate to style + } } else if (!rSprm.second->getSprms().empty() || !rSprm.second->getAttributes().empty()) { - RTFSprms const sprms( - pValue->getSprms().cloneAndDeduplicate(rSprm.second->getSprms(), nStyleType)); + RTFSprms const sprms(pValue->getSprms().cloneAndDeduplicate( + rSprm.second->getSprms(), nStyleType, /*bImplicitPPr =*/false, pDirect)); RTFSprms const attributes(pValue->getAttributes().cloneAndDeduplicate( - rSprm.second->getAttributes(), nStyleType)); + rSprm.second->getAttributes(), nStyleType, /*bImplicitPPr =*/false, pDirect)); // Don't copy the sprm in case we expect it to have children but it doesn't have some. if (!isSPRMChildrenExpected(rSprm.first) || !sprms.empty() || !attributes.empty()) ret.set(rSprm.first, @@ -377,7 +384,7 @@ void RTFSprms::duplicateList(const RTFValue::Pointer_t& pAbstract) } RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference, Id const nStyleType, - bool const bImplicitPPr) const + bool const bImplicitPPr, RTFSprms* pDirect) const { RTFSprms ret(*this); ret.ensureCopyBeforeWrite(); @@ -393,10 +400,10 @@ RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference, Id const nStyleType if (bImplicitPPr && rSprm.first == NS_ooxml::LN_CT_Style_pPr) { for (const auto& i : rSprm.second->getSprms()) - cloneAndDeduplicateSprm(i, ret, nStyleType); + cloneAndDeduplicateSprm(i, ret, nStyleType, pDirect); } else - cloneAndDeduplicateSprm(rSprm, ret, nStyleType); + cloneAndDeduplicateSprm(rSprm, ret, nStyleType, pDirect); } return ret; } diff --git a/writerfilter/source/rtftok/rtfsprm.hxx b/writerfilter/source/rtftok/rtfsprm.hxx index 1b489fcd5864..a84e57642c0f 100644 --- a/writerfilter/source/rtftok/rtfsprm.hxx +++ b/writerfilter/source/rtftok/rtfsprm.hxx @@ -61,8 +61,9 @@ public: /// Also insert default values to override attributes of style /// (yes, really; that's what Word does). /// @param bImplicitPPr implicit dereference of top-level pPr SPRM - RTFSprms cloneAndDeduplicate(RTFSprms& rReference, Id nStyleType, - bool bImplicitPPr = false) const; + /// @param pDirect pointer to the root of the direct formatting SPRM tree, if any + RTFSprms cloneAndDeduplicate(RTFSprms& rReference, Id nStyleType, bool bImplicitPPr = false, + RTFSprms* pDirect = nullptr) const; /// Inserts default values to override attributes of pAbstract. void duplicateList(const RTFValue::Pointer_t& pAbstract); /// Removes duplicated values based on in-list properties. |