[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - writerfilter/CppunitTest_writerfilter_rtftok.mk writerfilter/qa writerfilter/source
Miklos Vajna (via logerrit)
logerrit at kemper.freedesktop.org
Wed Oct 14 08:33:49 UTC 2020
writerfilter/CppunitTest_writerfilter_rtftok.mk | 1
writerfilter/qa/cppunittests/rtftok/data/left-margin-dedup.rtf | 26 +++
writerfilter/qa/cppunittests/rtftok/rtfsprm.cxx | 83 ++++++++++
writerfilter/source/rtftok/rtfdocumentimpl.cxx | 2
writerfilter/source/rtftok/rtfsprm.cxx | 25 +--
writerfilter/source/rtftok/rtfsprm.hxx | 5
6 files changed, 130 insertions(+), 12 deletions(-)
New commits:
commit b2c484c287ecd6c82822a2a672ee2193eba191df
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon Oct 12 21:04:36 2020 +0200
Commit: Caolán McNamara <caolanm at redhat.com>
CommitDate: Wed Oct 14 10:33:16 2020 +0200
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.
(cherry picked from commit 3ee3ae85de3a29ebfb89e75960b65417bfd6ca55)
Conflicts:
writerfilter/source/rtftok/rtfsprm.cxx
Change-Id: I5e27502b8d505bdfddfdc910858f62e501db35a5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104224
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
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 97844308de0b..f470ac552fad 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 fe6af8e5e4d8..5c2edbb1c65a 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 isSPRMDeduplicateBlacklist(Id nId)
+static bool isSPRMDeduplicateBlacklist(Id nId, RTFSprms* pDirect)
{
switch (nId)
{
@@ -223,6 +223,11 @@ static bool isSPRMDeduplicateBlacklist(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 (!isSPRMDeduplicateBlacklist(rSprm.first))
+ if (!isSPRMDeduplicateBlacklist(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 dd3e05cb8723..bb4f7c790819 100644
--- a/writerfilter/source/rtftok/rtfsprm.hxx
+++ b/writerfilter/source/rtftok/rtfsprm.hxx
@@ -63,8 +63,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.
More information about the Libreoffice-commits
mailing list