sw/qa/extras/ooxmlexport/data/sdt_after_section_break.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport20.cxx | 50 +++++++++++++
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 36 +++++----
3 files changed, 72 insertions(+), 14 deletions(-)
New commits:
commit 1e8e2134c66b2875aeacddfc9c68fdd584f74892
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Jan 2 17:12:06 2024 +0600
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Mon Jan 8 09:50:15 2024 +0100
tdf#158971: Only copy directly-set properties
Commit f09420fa189be5165b0311083ba127073500a121 (tdf#158855: Make sure
to not add extra paragraph after a table in a section, 2023-12-25) had
implemented copying all attributes of a paragraph using copyAllProps.
But the function didn't check the states of properties, so copied also
default values of properties, making them direct properties of the
paragraph. Due to tdf#158972, that caused an assertion when saving,
and the main problem was a set of direct properties in the paragraph.
Fix that by checking the properties states. Also make sure to work with
paragraphs, rather than with ranges, which treats any properties set on
paragraph level as default-state on the run level.
Change-Id: I7cd9c7fbb9313d666c46be201913f0223a6b4f5e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161539
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161580
diff --git a/sw/qa/extras/ooxmlexport/data/sdt_after_section_break.docx
b/sw/qa/extras/ooxmlexport/data/sdt_after_section_break.docx
new file mode 100644
index 000000000000..d753f5da30bf
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/sdt_after_section_break.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
index 639aea237618..317937fb3742 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport20.cxx
@@ -9,6 +9,7 @@
#include <swmodeltestbase.hxx>
+#include <com/sun/star/beans/XPropertyState.hpp>
#include <com/sun/star/text/XDocumentIndex.hpp>
#include <com/sun/star/text/XTextTable.hpp>
#include <com/sun/star/style/LineSpacing.hpp>
@@ -1041,6 +1042,55 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf158855)
getParagraph(2, u"Next page"_ustr);
}
+CPPUNIT_TEST_FIXTURE(Test, testTdf158971)
+{
+ // Given a section break and an SDT in the following paragraph
+ load(createFileURL(u"sdt_after_section_break.docx"));
+
+ // Check that the import doesn't introduce unwanted character properties
in the paragraph after
+ // the section break
+ CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
+ {
+ auto para = getParagraph(2, u"text"_ustr);
+ css::uno::Reference<css::beans::XPropertyState> xRunState(getRun(para,
1, u""_ustr),
+
css::uno::UNO_QUERY_THROW);
+ // without the fix, this would fail with
+ // - Expected: 1
+ // - Actual : 0
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+ xRunState->getPropertyState(u"RubyAdjust"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+ xRunState->getPropertyState(u"RubyIsAbove"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+
xRunState->getPropertyState(u"RubyPosition"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+
xRunState->getPropertyState(u"UnvisitedCharStyleName"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+
xRunState->getPropertyState(u"VisitedCharStyleName"_ustr));
+ }
+
+ // Saving must not fail assertions
+ saveAndReload(mpFilter);
+
+ // Check again
+ CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
+ {
+ auto para = getParagraph(2, u"text"_ustr);
+ css::uno::Reference<css::beans::XPropertyState> xRunState(getRun(para,
1, u""_ustr),
+
css::uno::UNO_QUERY_THROW);
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+ xRunState->getPropertyState(u"RubyAdjust"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+ xRunState->getPropertyState(u"RubyIsAbove"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+
xRunState->getPropertyState(u"RubyPosition"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+
xRunState->getPropertyState(u"UnvisitedCharStyleName"_ustr));
+ CPPUNIT_ASSERT_EQUAL(css::beans::PropertyState_DEFAULT_VALUE,
+
xRunState->getPropertyState(u"VisitedCharStyleName"_ustr));
+ }
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 11d58a3b2031..db2a71270331 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -3519,8 +3519,6 @@ static void checkAndAddPropVal(const OUString& prop,
const css::uno::Any& val,
// Avoid well-known reasons for exceptions when setting property values
if (!val.hasValue())
return;
- if (prop == "CharAutoStyleName" || prop == "ParaAutoStyleName")
- return;
if (prop == "CharStyleName" || prop == "DropCapCharStyleName")
if (OUString val_string; (val >>= val_string) && val_string.isEmpty())
return;
@@ -3529,6 +3527,13 @@ static void checkAndAddPropVal(const OUString& prop,
const css::uno::Any& val,
values.push_back(val);
}
+static uno::Reference<lang::XComponent>
+getParagraphOfRange(const css::uno::Reference<css::text::XTextRange>& xRange)
+{
+ uno::Reference<container::XEnumerationAccess> xEA{ xRange,
uno::UNO_QUERY_THROW };
+ return { xEA->createEnumeration()->nextElement(), uno::UNO_QUERY_THROW };
+}
+
static void copyAllProps(const css::uno::Reference<css::uno::XInterface>& from,
const css::uno::Reference<css::uno::XInterface>& to)
{
@@ -3541,6 +3546,17 @@ static void copyAllProps(const
css::uno::Reference<css::uno::XInterface>& from,
for (const auto& prop : rawProps)
if ((prop.Attributes & css::beans::PropertyAttribute::READONLY) == 0)
props.push_back(prop.Name);
+
+ if (css::uno::Reference<css::beans::XPropertyState> xFromState{ from,
css::uno::UNO_QUERY })
+ {
+ const auto propsSeq = comphelper::containerToSequence(props);
+ const auto statesSeq = xFromState->getPropertyStates(propsSeq);
+ assert(propsSeq.getLength() == statesSeq.getLength());
+ for (sal_Int32 i = 0; i < propsSeq.getLength(); ++i)
+ if (statesSeq[i] != css::beans::PropertyState_DIRECT_VALUE)
+ std::erase(props, propsSeq[i]);
+ }
+
std::vector<css::uno::Any> values;
values.reserve(props.size());
if (css::uno::Reference<css::beans::XMultiPropertySet> xFromMulti{
xFromProps,
@@ -3616,12 +3632,11 @@ uno::Reference< beans::XPropertySet >
DomainMapper_Impl::appendTextSectionAfter(
// table; then trying to go left would skip the whole table. Split
the trailing
// paragraph; let the section span over the first of the two
resulting paragraphs;
// destroy the last section's paragraph afterwards.
- css::uno::Reference<css::text::XTextRange> xEndPara =
xCursor->getEnd();
xTextAppend->insertControlCharacter(
- xEndPara, css::text::ControlCharacter::PARAGRAPH_BREAK, false);
- css::uno::Reference<css::text::XTextRange> xNewPara =
xCursor->getEnd();
+ xCursor->getEnd(),
css::text::ControlCharacter::PARAGRAPH_BREAK, false);
+ auto xNewPara = getParagraphOfRange(xCursor->getEnd());
xCursor->gotoPreviousParagraph(true);
- xEndPara = xCursor->getEnd();
+ auto xEndPara = getParagraphOfRange(xCursor->getEnd());
// xEndPara may already have properties (like page break); make
sure to apply them
// to the newly appended paragraph, which will be kept in the end.
copyAllProps(xEndPara, xNewPara);
@@ -3630,14 +3645,7 @@ uno::Reference< beans::XPropertySet >
DomainMapper_Impl::appendTextSectionAfter(
xSection->attach(xCursor);
// Remove the extra paragraph (last inside the section)
- if (uno::Reference<container::XEnumerationAccess> xEA{ xEndPara,
uno::UNO_QUERY })
- {
- if (uno::Reference<lang::XComponent> xParagraph{
- xEA->createEnumeration()->nextElement(),
uno::UNO_QUERY })
- {
- xParagraph->dispose();
- }
- }
+ xEndPara->dispose();
xRet.set(xSection, uno::UNO_QUERY );
}
