sw/qa/extras/rtfexport/data/tdf167185.rtf | 8 +++
sw/qa/extras/rtfexport/rtfexport8.cxx | 52 ++++++++++++++++++++++
sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx | 26 ++++++-----
sw/source/writerfilter/rtftok/rtfsprm.cxx | 6 ++
sw/source/writerfilter/rtftok/rtfsprm.hxx | 3 +
5 files changed, 84 insertions(+), 11 deletions(-)
New commits:
commit 07fa13b3319d2d5bc2ddf2f6bdbf5167db0d9936
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Tue Jun 24 16:38:28 2025 +0500
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Jun 24 17:22:08 2025 +0200
tdf#167185: make sure that cell width corrections don't accumulate
The problem was, that several rows may share the same row definition,
and the same RTFSprms in the current state. In commit
d00181e8c1ead5020cc8fee47ee7af7fc5039552 (tdf#167169: postpone rleftN
processing until ow, 2025-06-23), RTFDocumentImpl::prepareProperties
started to modify existing values inside table row sprms in place; and
that resulted in accumulating changes, where each next row was getting
more corrections of already corrected widths.
This change copies the RTFSprms for correction, avoiding the problem.
Change-Id: Ia199862c7b8983d93d2fce4898fb60bf8b4a1a4f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186890
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/sw/qa/extras/rtfexport/data/tdf167185.rtf
b/sw/qa/extras/rtfexport/data/tdf167185.rtf
new file mode 100644
index 000000000000..d0be00a353b0
--- /dev/null
+++ b/sw/qa/extras/rtfexport/data/tdf167185.rtf
@@ -0,0 +1,8 @@
+{ tf1
+ rowd rleft1260+\pard\intbl A1+\pard\intbl A2+\pard\intbl
A3+\pard\intbl A4+\pard\intbl A5+}
\ No newline at end of file
diff --git a/sw/qa/extras/rtfexport/rtfexport8.cxx
b/sw/qa/extras/rtfexport/rtfexport8.cxx
index 90b389e0e1f3..d651d291be75 100644
--- a/sw/qa/extras/rtfexport/rtfexport8.cxx
+++ b/sw/qa/extras/rtfexport/rtfexport8.cxx
@@ -852,6 +852,58 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf167169)
}
}
+CPPUNIT_TEST_FIXTURE(Test, testTdf167185)
+{
+ // Given a document with a table with a single rleft1260+ // and
five rows, all sharing the same row data:
+ createSwDoc("tdf167185.rtf");
+ {
+ xmlDocUniquePtr pLayout = parseLayoutDump();
+ assertXPath(pLayout, "//tab['pass 1']", 1);
+ assertXPath(pLayout, "//tab['pass 1']/row", 5);
+ assertXPath(pLayout, "//tab['pass 1']/row[1]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 1']/row[1]/cell[1]/infos/bounds",
"width", u"3119");
+ assertXPath(pLayout, "//tab['pass 1']/row[1]/cell[2]/infos/bounds",
"width", u"1517");
+ assertXPath(pLayout, "//tab['pass 1']/row[2]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 1']/row[2]/cell[1]/infos/bounds",
"width", u"3119");
+ assertXPath(pLayout, "//tab['pass 1']/row[2]/cell[2]/infos/bounds",
"width", u"1517");
+ assertXPath(pLayout, "//tab['pass 1']/row[3]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 1']/row[3]/cell[1]/infos/bounds",
"width", u"3119");
+ assertXPath(pLayout, "//tab['pass 1']/row[3]/cell[2]/infos/bounds",
"width", u"1517");
+ assertXPath(pLayout, "//tab['pass 1']/row[4]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 1']/row[4]/cell[1]/infos/bounds",
"width", u"3119");
+ assertXPath(pLayout, "//tab['pass 1']/row[4]/cell[2]/infos/bounds",
"width", u"1517");
+ assertXPath(pLayout, "//tab['pass 1']/row[5]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 1']/row[5]/cell[1]/infos/bounds",
"width", u"3119");
+ assertXPath(pLayout, "//tab['pass 1']/row[5]/cell[2]/infos/bounds",
"width", u"1517");
+ }
+ // Check export, too
+ saveAndReload(mpFilter);
+ {
+ xmlDocUniquePtr pLayout = parseLayoutDump();
+ assertXPath(pLayout, "//tab['pass 2']", 1);
+ assertXPath(pLayout, "//tab['pass 2']/row", 5);
+ assertXPath(pLayout, "//tab['pass 2']/row[1]/cell", 2);
+ // Rounding (or maybe off-by-one?) errors sadly hit the test
+ OUString width1 = getXPath(pLayout, "//tab['pass
2']/row[1]/cell[1]/infos/bounds", "width");
+ CPPUNIT_ASSERT_DOUBLES_EQUAL(3119, width1.toInt32(), 1);
+ OUString width2 = getXPath(pLayout, "//tab['pass
2']/row[1]/cell[2]/infos/bounds", "width");
+ CPPUNIT_ASSERT_DOUBLES_EQUAL(1517, width2.toInt32(), 1);
+ assertXPath(pLayout, "//tab['pass 2']/row[2]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 2']/row[2]/cell[1]/infos/bounds",
"width", width1);
+ assertXPath(pLayout, "//tab['pass 2']/row[2]/cell[2]/infos/bounds",
"width", width2);
+ assertXPath(pLayout, "//tab['pass 2']/row[3]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 2']/row[3]/cell[1]/infos/bounds",
"width", width1);
+ assertXPath(pLayout, "//tab['pass 2']/row[3]/cell[2]/infos/bounds",
"width", width2);
+ assertXPath(pLayout, "//tab['pass 2']/row[4]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 2']/row[4]/cell[1]/infos/bounds",
"width", width1);
+ assertXPath(pLayout, "//tab['pass 2']/row[4]/cell[2]/infos/bounds",
"width", width2);
+ assertXPath(pLayout, "//tab['pass 2']/row[5]/cell", 2);
+ assertXPath(pLayout, "//tab['pass 2']/row[5]/cell[1]/infos/bounds",
"width", width1);
+ assertXPath(pLayout, "//tab['pass 2']/row[5]/cell[2]/infos/bounds",
"width", width2);
+ }
+}
+
} // end of anonymous namespace
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx
b/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx
index b7982553bc7c..0109364df396 100644
--- a/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx
+++ b/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx
@@ -1748,16 +1748,20 @@ void RTFDocumentImpl::prepareProperties(
o_rpFrameProperties = new RTFReferenceProperties(RTFSprms(),
rState.getFrame().getSprms());
}
+ // prepareProperties may be called several times for the same rState (once
per row); to avoid
+ // applying the same cell width correction several times, copy
TableRowSprms for modification
+ RTFSprms localTableRowSprms(rState.getTableRowSprms(),
RTFSprms::CopyForWrite());
+
// Table width.
RTFValue::Pointer_t const pTableWidthProps
- = rState.getTableRowSprms().find(NS_ooxml::LN_CT_TblPrBase_tblW);
+ = localTableRowSprms.find(NS_ooxml::LN_CT_TblPrBase_tblW);
if (!pTableWidthProps)
{
auto pUnitValue = new RTFValue(3);
- putNestedAttribute(rState.getTableRowSprms(),
NS_ooxml::LN_CT_TblPrBase_tblW,
+ putNestedAttribute(localTableRowSprms, NS_ooxml::LN_CT_TblPrBase_tblW,
NS_ooxml::LN_CT_TblWidth_type, pUnitValue);
auto pWValue = new RTFValue(nCurrentCellX - nTRLeft);
- putNestedAttribute(rState.getTableRowSprms(),
NS_ooxml::LN_CT_TblPrBase_tblW,
+ putNestedAttribute(localTableRowSprms, NS_ooxml::LN_CT_TblPrBase_tblW,
NS_ooxml::LN_CT_TblWidth_w, pWValue);
}
@@ -1765,7 +1769,7 @@ void RTFDocumentImpl::prepareProperties(
bool checkedMinusOne = false;
bool seenFirstColumn = false;
bool seenPositiveWidth = false;
- for (auto & [ id, pValue ] : rState.getTableRowSprms())
+ for (auto & [ id, pValue ] : localTableRowSprms)
{
if (id == NS_ooxml::LN_CT_TblGridBase_gridCol)
{
@@ -1811,17 +1815,17 @@ void RTFDocumentImpl::prepareProperties(
if (nTRLeft != 0)
{
// If there was no tblind, use trleft to set up LN_CT_TblPrBase_tblInd
- if (!rState.getTableRowSprms().find(NS_ooxml::LN_CT_TblPrBase_tblInd))
+ if (!localTableRowSprms.find(NS_ooxml::LN_CT_TblPrBase_tblInd))
{
- set_tblInd(rState.getTableRowSprms(), nTRLeft);
+ set_tblInd(localTableRowSprms, nTRLeft);
}
}
if (nCells > 0)
- rState.getTableRowSprms().set(NS_ooxml::LN_tblRow, new RTFValue(1));
+ localTableRowSprms.set(NS_ooxml::LN_tblRow, new RTFValue(1));
RTFValue::Pointer_t const pCellMar
- = rState.getTableRowSprms().find(NS_ooxml::LN_CT_TblPrBase_tblCellMar);
+ = localTableRowSprms.find(NS_ooxml::LN_CT_TblPrBase_tblCellMar);
if (!pCellMar)
{
// If no cell margins are defined, the default left/right margin is 0
in Word, but not in Writer.
@@ -1829,14 +1833,14 @@ void RTFDocumentImpl::prepareProperties(
aAttributes.set(NS_ooxml::LN_CT_TblWidth_type,
new RTFValue(NS_ooxml::LN_Value_ST_TblWidth_dxa));
aAttributes.set(NS_ooxml::LN_CT_TblWidth_w, new RTFValue(0));
- putNestedSprm(rState.getTableRowSprms(),
NS_ooxml::LN_CT_TblPrBase_tblCellMar,
+ putNestedSprm(localTableRowSprms, NS_ooxml::LN_CT_TblPrBase_tblCellMar,
NS_ooxml::LN_CT_TblCellMar_left, new
RTFValue(aAttributes));
- putNestedSprm(rState.getTableRowSprms(),
NS_ooxml::LN_CT_TblPrBase_tblCellMar,
+ putNestedSprm(localTableRowSprms, NS_ooxml::LN_CT_TblPrBase_tblCellMar,
NS_ooxml::LN_CT_TblCellMar_right, new
RTFValue(aAttributes));
}
o_rpTableRowProperties
- = new RTFReferenceProperties(rState.getTableRowAttributes(),
rState.getTableRowSprms());
+ = new RTFReferenceProperties(rState.getTableRowAttributes(),
localTableRowSprms);
}
void RTFDocumentImpl::sendProperties(
diff --git a/sw/source/writerfilter/rtftok/rtfsprm.cxx
b/sw/source/writerfilter/rtftok/rtfsprm.cxx
index 8549d54e41e3..830bd3bb8e05 100644
--- a/sw/source/writerfilter/rtftok/rtfsprm.cxx
+++ b/sw/source/writerfilter/rtftok/rtfsprm.cxx
@@ -481,6 +481,12 @@ RTFSprms::RTFSprms()
RTFSprms::~RTFSprms() = default;
+RTFSprms::RTFSprms(RTFSprms const& source, CopyForWrite)
+ : RTFSprms(source)
+{
+ ensureCopyBeforeWrite();
+}
+
void RTFSprms::clear()
{
if (m_pSprms->GetRefCount() == 1)
diff --git a/sw/source/writerfilter/rtftok/rtfsprm.hxx
b/sw/source/writerfilter/rtftok/rtfsprm.hxx
index 275048b4abc1..344c61a42e66 100644
--- a/sw/source/writerfilter/rtftok/rtfsprm.hxx
+++ b/sw/source/writerfilter/rtftok/rtfsprm.hxx
@@ -45,6 +45,9 @@ public:
RTFSprms();
~RTFSprms() override;
+ enum class CopyForWrite;
+ RTFSprms(RTFSprms const& source, CopyForWrite);
+
RTFSprms(RTFSprms const&) = default;
RTFSprms(RTFSprms&&) = default;
RTFSprms& operator=(RTFSprms const&) = default;
