summaryrefslogtreecommitdiff
path: root/writerfilter
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2020-11-09 21:05:55 +0100
committerMiklos Vajna <vmiklos@collabora.com>2020-11-10 09:03:18 +0100
commit0afba07a597bf1d361624e10968855a802b859a0 (patch)
tree1dac8441eb72e8da460e2797764c7814400e46dc /writerfilter
parent7e2c35324c54646f53f0fa14b7bce07e1da73c0b (diff)
DOCX import: fix <w:spacing w:before="..."/> for more than 58cm
Regression from commit af313fc149f80adb0f1680ca20e19745ccb7fede (tdf#105143 DOCX import: enable DoNotCaptureDrawObjsOnPage layout compat option, 2017-01-06), this now uncovered a deeper problem that paragraph top margin import goes through ConversionHelper::convertTwipToMM100(), which ignores values larger than 0x8000. Previously this problem was not visible as we (incorrectly) captured draw objects inside the page frame. Word does not ignore values larger than that constant for paragraphs, so fix the problem by using convertTwipToMm100() directly to just do the conversion, without any checks on the input. And now that the paragraph margin is not lost, it'll have the correct horizontal position, so the position of the triangle in the bugdoc will be correct, too. Change-Id: If664ad055f1916b7e7fb2fb85d1afa977c2d03aa Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105496 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Diffstat (limited to 'writerfilter')
-rw-r--r--writerfilter/CppunitTest_writerfilter_dmapper.mk1
-rw-r--r--writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx82
-rw-r--r--writerfilter/qa/cppunittests/dmapper/data/large-para-top-margin.docxbin0 -> 23126 bytes
-rw-r--r--writerfilter/source/dmapper/DomainMapper.cxx5
4 files changed, 87 insertions, 1 deletions
diff --git a/writerfilter/CppunitTest_writerfilter_dmapper.mk b/writerfilter/CppunitTest_writerfilter_dmapper.mk
index c29b3b9e1c39..7fe8b9035d3d 100644
--- a/writerfilter/CppunitTest_writerfilter_dmapper.mk
+++ b/writerfilter/CppunitTest_writerfilter_dmapper.mk
@@ -18,6 +18,7 @@ $(eval $(call gb_CppunitTest_use_externals,writerfilter_dmapper,\
$(eval $(call gb_CppunitTest_add_exception_objects,writerfilter_dmapper, \
writerfilter/qa/cppunittests/dmapper/CellColorHandler \
writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler \
+ writerfilter/qa/cppunittests/dmapper/DomainMapper \
writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl \
writerfilter/qa/cppunittests/dmapper/GraphicImport \
writerfilter/qa/cppunittests/dmapper/TextEffectsHandler \
diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx
new file mode 100644
index 000000000000..d6e6830b88fb
--- /dev/null
+++ b/writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx
@@ -0,0 +1,82 @@
+/* -*- 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/frame/Desktop.hpp>
+#include <com/sun/star/text/XTextDocument.hpp>
+#include <com/sun/star/beans/XPropertySet.hpp>
+#include <com/sun/star/style/BreakType.hpp>
+#include <com/sun/star/drawing/XDrawPageSupplier.hpp>
+#include <com/sun/star/text/WritingMode2.hpp>
+
+#include <tools/UnitConversion.hxx>
+
+using namespace ::com::sun::star;
+
+namespace
+{
+/// Tests for writerfilter/source/dmapper/DomainMapper.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/dmapper/data/";
+
+CPPUNIT_TEST_FIXTURE(Test, testLargeParaTopMargin)
+{
+ // Given a document with a paragraph with a large "before" spacing.
+ OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "large-para-top-margin.docx";
+ getComponent() = loadFromDesktop(aURL);
+
+ // When checking the first paragraph.
+ uno::Reference<text::XTextDocument> xTextDocument(getComponent(), uno::UNO_QUERY);
+ uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xTextDocument->getText(),
+ uno::UNO_QUERY);
+ uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration();
+ uno::Reference<beans::XPropertySet> xPara(xParaEnum->nextElement(), uno::UNO_QUERY);
+
+ // Then assert its top margin.
+ sal_Int32 nParaTopMargin{};
+ xPara->getPropertyValue("ParaTopMargin") >>= nParaTopMargin;
+ // <w:spacing w:before="37050"/> in the document.
+ sal_Int32 nExpected = convertTwipToMm100(37050);
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 65352
+ // - Actual : 0
+ // i.e. the paragraph margin was lost, which shifted the paragraph to the right (no top margin
+ // -> wrap around a TextBox), which shifted the triangle shape out of the page frame.
+ CPPUNIT_ASSERT_EQUAL(nExpected, nParaTopMargin);
+}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/qa/cppunittests/dmapper/data/large-para-top-margin.docx b/writerfilter/qa/cppunittests/dmapper/data/large-para-top-margin.docx
new file mode 100644
index 000000000000..34f24a3e2a12
--- /dev/null
+++ b/writerfilter/qa/cppunittests/dmapper/data/large-para-top-margin.docx
Binary files differ
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index cb46a925abab..aac420a97500 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -84,6 +84,7 @@
#include <dmapper/GraphicZOrderHelper.hxx>
#include <tools/diagnose_ex.h>
#include <sal/log.hxx>
+#include <tools/UnitConversion.hxx>
using namespace ::com::sun::star;
using namespace oox;
@@ -417,7 +418,9 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
m_pImpl->appendGrabBag(m_pImpl->m_aSubInteropGrabBag, "before", OUString::number(nIntValue));
if (m_pImpl->GetTopContext())
// Don't overwrite NS_ooxml::LN_CT_Spacing_beforeAutospacing.
- m_pImpl->GetTopContext()->Insert(PROP_PARA_TOP_MARGIN, uno::makeAny( ConversionHelper::convertTwipToMM100( nIntValue ) ), false);
+ m_pImpl->GetTopContext()->Insert(
+ PROP_PARA_TOP_MARGIN,
+ uno::makeAny(static_cast<sal_Int32>(convertTwipToMm100(nIntValue))), false);
break;
case NS_ooxml::LN_CT_Spacing_beforeLines:
m_pImpl->appendGrabBag(m_pImpl->m_aSubInteropGrabBag, "beforeLines", OUString::number(nIntValue));