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.kaganski@collabora.com>
(cherry picked from commit b8ccf3dacb19e157d9b2c35cc2b083cb39e76da0)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153422
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf155945.docx b/sw/qa/extras/ooxmlexport/data/tdf155945.docx
new file mode 100644
index 0000000..9858ac7
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf155945.docx
Binary files differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx
index 2a4e3ee..ae45bc3 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx
@@ -1006,6 +1006,17 @@ DECLARE_OOXMLEXPORT_TEST(testTdf131722, "tdf131722.docx")
}
}
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 ccbcc53..55baec8 100644
--- a/sw/source/core/unocore/unoobj.cxx
+++ b/sw/source/core/unocore/unoobj.cxx
@@ -1927,6 +1927,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)
@@ -1947,39 +1948,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())
