sw/qa/extras/ooxmlexport/data/tdf155945.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport18.cxx | 11 ++++++
sw/source/core/unocore/unoobj.cxx | 48 +++++++++++++++------------
3 files changed, 38 insertions(+), 21 deletions(-)
New commits:
commit 8afe5d3ad14932884097ccdc8e28e35b97d9b259
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Wed Jun 21 13:38:14 2023 +0300
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Wed Jun 21 23:02:14 2023 +0200
tdf#155945: do not turn another style attributes to direct formatting
SwUnoCursorHelper::SetPropertyValues obtains the node's item set using
SwUnoCursorHelper::GetCursorAttr, adds new items for the property values,
and calls SwUnoCursorHelper::SetCursorAttr to set the updated item set
to the node. This puts all the item set data as direct formatting of the
node (autostyle).
For complex SfxPoolItem, containing several properties, it could result
in some properties having incorrect values e.g. in this sequence:
1. SwUnoCursorHelper::SetPropertyValues is called with a property sequence
including "ParaStyleName" and "ParaTopMargin", represented by a complex
SfxPoolItem (SvxULSpaceItem's nPropUpper). The current node uses another
style, which has a non-default bottom margin; new style has 0 as bottom
margin.
2. It builds a set for the union of all WhichIds corresponding to all the
passed properties, which includes WIDs for paragraph style and for the
respective SvxULSpaceItem.
3. It calls SwUnoCursorHelper::GetCursorAttr to fill the set with current
values for these WhichIds, getting also values defined in still applied
style, including an SvxULSpaceItem with both upper and lower spacing
(the bottom margin value of old paragraph is stored there).
4. It sets new value of paragraph style in this item set, and then calls
SwUnoCursorHelper::SetCursorAttr with the item set.
Now the set contains a new value for paragraph style, *and* an old value
of the paragraphs margins. This margins value is set as the paragraph's
direct formatting (autostyle).
5. SwUnoCursorHelper::GetCursorAttr is called, updating the item set with
new values of paragraph style and margins; the direct-formatted value
for the latter is returned, still containing the values previously
inherited from old style.
6. New value of top margin is set, modifying only nPropUpper member of
SvxULSpaceItem, and keeping nPropLower (value for bottom margin) as it
was, i.e. keeps the value incorrectly saved from old style.
This happens e.g. on DOCX import. Commit
db115bec9254417ef7a3faf687478fe5424ab378
(tdf#78510 sw,cui: split SvxLRSpaceItem for SwTextNode, SwTextFormatColl,
2023-02-24) has incidentally fixed handling of left and right paragraph
indents, making their items independent; but top/bottom margins still need
to be fixed.
This change splits the properties that cause side effects (styles) from the
rest; so that properties with side effects are processed first, with item
sets containing only one WID for the currently processed property. This
both allows to avoid the problem, and simplifies the code.
Change-Id: Idd451fa1e53806cdb6b9064dec31e9171d14d5d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153384
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153408
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf155945.docx
b/sw/qa/extras/ooxmlexport/data/tdf155945.docx
new file mode 100644
index 000000000000..9858ac711790
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf155945.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
index 00fbebcb3f4e..e686e1c3dd02 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
@@ -587,6 +587,17 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf153128)
CPPUNIT_ASSERT_LESS(sal_Int32(30), nFirstLineHeight);
}
+CPPUNIT_TEST_FIXTURE(Test, testTdf155945)
+{
+ createSwDoc("tdf155945.docx");
+
+ CPPUNIT_ASSERT_EQUAL(3, getParagraphs());
+ // Without a fix in place, this would fail with
+ // - Expected: 0
+ // - Actual : 423
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(getParagraph(2),
"ParaBottomMargin"));
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/unocore/unoobj.cxx
b/sw/source/core/unocore/unoobj.cxx
index 4315d173796d..146c568ef009 100644
--- a/sw/source/core/unocore/unoobj.cxx
+++ b/sw/source/core/unocore/unoobj.cxx
@@ -1934,6 +1934,7 @@ void SwUnoCursorHelper::SetPropertyValues(
// Build set of attributes we want to fetch
WhichRangesContainer aRanges;
+ std::vector<std::pair<const SfxItemPropertyMapEntry*, const uno::Any&>>
aSideEffectsEntries;
std::vector<std::pair<const SfxItemPropertyMapEntry*, const uno::Any&>>
aEntries;
aEntries.reserve(aPropertyValues.size());
for (const auto& rPropVal : aPropertyValues)
@@ -1954,39 +1955,44 @@ void SwUnoCursorHelper::SetPropertyValues(
aPropertyVetoExMsg += "Property is read-only: '" + rPropertyName +
"' ";
continue;
}
- aRanges = aRanges.MergeRange(pEntry->nWID, pEntry->nWID);
- aEntries.emplace_back(pEntry, rPropVal.Value);
+ if (propertyCausesSideEffectsInNodes(pEntry->nWID))
+ {
+ aSideEffectsEntries.emplace_back(pEntry, rPropVal.Value);
+ }
+ else
+ {
+ aRanges = aRanges.MergeRange(pEntry->nWID, pEntry->nWID);
+ aEntries.emplace_back(pEntry, rPropVal.Value);
+ }
+ }
+
+ // Entries with side effects first, using dedicated one-element SfxItemSet
for each
+ for (const auto& [pEntry, rValue] : aSideEffectsEntries)
+ {
+ SfxItemSet aItemSet(rDoc.GetAttrPool(), pEntry->nWID, pEntry->nWID);
+ // we need to get up-to-date item set from nodes
+ SwUnoCursorHelper::GetCursorAttr(rPaM, aItemSet);
+ // this can set some attributes in nodes' mpAttrSet
+ if (!SwUnoCursorHelper::SetCursorPropertyValue(*pEntry, rValue, rPaM,
aItemSet))
+ rPropSet.setPropertyValue(*pEntry, rValue, aItemSet);
+ SwUnoCursorHelper::SetCursorAttr(rPaM, aItemSet, nAttrMode, false
/*bTableMode*/);
}
if (!aEntries.empty())
{
// Fetch, overwrite, and re-set the attributes from the core
SfxItemSet aItemSet(rDoc.GetAttrPool(), std::move(aRanges));
+ // we need to get up-to-date item set from nodes
+ SwUnoCursorHelper::GetCursorAttr(rPaM, aItemSet);
- bool bPreviousPropertyCausesSideEffectsInNodes = false;
- for (size_t i = 0; i < aEntries.size(); ++i)
+ for (const auto& [pEntry, rValue] : aEntries)
{
- SfxItemPropertyMapEntry const*const pEntry = aEntries[i].first;
- bool bPropertyCausesSideEffectsInNodes =
- propertyCausesSideEffectsInNodes(pEntry->nWID);
-
- // we need to get up-to-date item set from nodes
- if (i == 0 || bPreviousPropertyCausesSideEffectsInNodes)
- {
- aItemSet.ClearItem();
- SwUnoCursorHelper::GetCursorAttr(rPaM, aItemSet);
- }
-
- const uno::Any &rValue = aEntries[i].second;
// this can set some attributes in nodes' mpAttrSet
if (!SwUnoCursorHelper::SetCursorPropertyValue(*pEntry, rValue,
rPaM, aItemSet))
rPropSet.setPropertyValue(*pEntry, rValue, aItemSet);
-
- if (i + 1 == aEntries.size() || bPropertyCausesSideEffectsInNodes)
- SwUnoCursorHelper::SetCursorAttr(rPaM, aItemSet, nAttrMode,
false/*bTableMode*/);
-
- bPreviousPropertyCausesSideEffectsInNodes =
bPropertyCausesSideEffectsInNodes;
}
+
+ SwUnoCursorHelper::SetCursorAttr(rPaM, aItemSet, nAttrMode, false
/*bTableMode*/);
}
if (!aUnknownExMsg.isEmpty())
