diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2020-11-09 21:05:55 +0100 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2020-11-10 09:03:18 +0100 |
commit | 0afba07a597bf1d361624e10968855a802b859a0 (patch) | |
tree | 1dac8441eb72e8da460e2797764c7814400e46dc /writerfilter | |
parent | 7e2c35324c54646f53f0fa14b7bce07e1da73c0b (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.mk | 1 | ||||
-rw-r--r-- | writerfilter/qa/cppunittests/dmapper/DomainMapper.cxx | 82 | ||||
-rw-r--r-- | writerfilter/qa/cppunittests/dmapper/data/large-para-top-margin.docx | bin | 0 -> 23126 bytes | |||
-rw-r--r-- | writerfilter/source/dmapper/DomainMapper.cxx | 5 |
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 Binary files differnew file mode 100644 index 000000000000..34f24a3e2a12 --- /dev/null +++ b/writerfilter/qa/cppunittests/dmapper/data/large-para-top-margin.docx 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)); |