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 c0f2b94e176d163591d7e11ba5956744c2579f05 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Jun 24 16:38:28 2025 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Thu Jun 26 18:03:53 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 <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186940 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 61c52ad205da..8102c579fb80 100644 --- a/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx +++ b/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx @@ -1747,16 +1747,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); } @@ -1764,7 +1768,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) { @@ -1810,17 +1814,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. @@ -1828,14 +1832,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 f61ffb957240..9b2dd7febb96 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 c493fd6e3d4a..e0f0de61b7e2 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;
