sw/qa/extras/rtfexport/data/tdf167169.rtf | 29 ++++++ sw/qa/extras/rtfexport/rtfexport8.cxx | 57 +++++++++++++ sw/source/writerfilter/rtftok/rtfdispatchsymbol.cxx | 4 sw/source/writerfilter/rtftok/rtfdispatchvalue.cxx | 84 +------------------- sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx | 79 ++++++++++++++++++ sw/source/writerfilter/rtftok/rtfdocumentimpl.hxx | 3 6 files changed, 175 insertions(+), 81 deletions(-)
New commits: commit d00181e8c1ead5020cc8fee47ee7af7fc5039552 Author: Mike Kaganski <[email protected]> AuthorDate: Mon Jun 23 09:54:48 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Mon Jun 23 09:15:10 2025 +0200 tdf#167169: postpone rleftN processing until ow ... where all the data: all The blind establishes the left indent; rleft establishes what X coordinate will be at the leftmost table edge (i.e., blindN tells "shift table N twips to the right"; rleftN tells "assume that the coordinate of this table's leftmost position is X"). Only when there is no blindN, the rleftN also defines an indent. Handling all of that correctly is only possible at ow handler; especially because the RTF standard tells: <row> (<tbldef> <cell>+ <tbldef> ow) | (<tbldef> <cell>+ ow) | (<cell>+ <tbldef> ow) ... While Word 97 emitted the row properties (<tbldef>) at the beginning of the row, a reader should not assume that this is the case. Properties can be emitted at the end, and, in fact, Word 2002, Word 2003, and Word 2007 do this. Note: there is a problem of inconsistencies in the documentation and implementation of blindN wrt. units: there is blindtypeN, which should define units used in blindN, and it is documented to use 0 for "auto", 1 for "dxa" (=twips), 2 for "no width", and 3 for "pct" (fiftieths of a percent). However, in my testing, Word considered 3 as twips, and ignored blindN when blindtypeN had other values. In Writer, blindtypeN is not handled. This needs a separate fix. Change-Id: If5c47571ca005b248d72ac7ab688c358f4f18e52 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/186807 Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins diff --git a/sw/qa/extras/rtfexport/data/tdf167169.rtf b/sw/qa/extras/rtfexport/data/tdf167169.rtf new file mode 100644 index 000000000000..6b14ea4f044d --- /dev/null +++ b/sw/qa/extras/rtfexport/data/tdf167169.rtf @@ -0,0 +1,29 @@ +{ tf1 +{ rowd blind1000 blindtype3+A1: tblind1000 tblindtype3 ++B1 ++ ow} +\par +{ rowd rleft500 blind1000 blindtype3+A1: trleft500 tblind1000 tblindtype3 ++B1 ++ ow} +\par +{ rowd rleft1000 blind1000 blindtype3+A1: trleft1000 tblind1000 tblindtype3 ++B1 ++ ow} +\par +{ rowd rleft1500 blind1000 blindtype3+A1: trleft1500 tblind1000 tblindtype3 ++B1 ++ ow} +} \ No newline at end of file diff --git a/sw/qa/extras/rtfexport/rtfexport8.cxx b/sw/qa/extras/rtfexport/rtfexport8.cxx index 5675cf61910b..90b389e0e1f3 100644 --- a/sw/qa/extras/rtfexport/rtfexport8.cxx +++ b/sw/qa/extras/rtfexport/rtfexport8.cxx @@ -795,6 +795,63 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf121493) } } +CPPUNIT_TEST_FIXTURE(Test, testTdf167169) +{ + // Given a document with four tables, having different values for tblindN and trleftN: + createSwDoc("tdf167169.rtf"); + { + xmlDocUniquePtr pLayout = parseLayoutDump(); + assertXPath(pLayout, "//tab", 4); + // 1st table, having blind1000 blindtype3+ assertXPath(pLayout, "//tab[1]['pass 1']/infos/prtBounds", "left", u"1000"); + assertXPath(pLayout, "//tab[1]['pass 1']/row/cell[1]/infos/bounds", "width", u"8000"); + assertXPath(pLayout, "//tab[1]['pass 1']/row/cell[2]/infos/bounds", "width", u"1000"); + // 2nd table, having rleft500 blind1000 blindtype3+ assertXPath(pLayout, "//tab[2]['pass 1']/infos/prtBounds", "left", u"1000"); + assertXPath(pLayout, "//tab[2]['pass 1']/row/cell[1]/infos/bounds", "width", u"7500"); + assertXPath(pLayout, "//tab[2]['pass 1']/row/cell[2]/infos/bounds", "width", u"1000"); + // 3rd table, having rleft1000 blind1000 blindtype3+ assertXPath(pLayout, "//tab[3]['pass 1']/infos/prtBounds", "left", u"1000"); + assertXPath(pLayout, "//tab[3]['pass 1']/row/cell[1]/infos/bounds", "width", u"7000"); + assertXPath(pLayout, "//tab[3]['pass 1']/row/cell[2]/infos/bounds", "width", u"1000"); + // 4th table, having rleft1500 blind1000 blindtype3+ assertXPath(pLayout, "//tab[4]['pass 1']/infos/prtBounds", "left", u"1000"); + assertXPath(pLayout, "//tab[4]['pass 1']/row/cell[1]/infos/bounds", "width", u"6500"); + assertXPath(pLayout, "//tab[4]['pass 1']/row/cell[2]/infos/bounds", "width", u"1000"); + } + // Check export, too + saveAndReload(mpFilter); + { + xmlDocUniquePtr pLayout = parseLayoutDump(); + assertXPath(pLayout, "//tab", 4); + // Rounding (or maybe off-by-one?) errors sadly hit the test + // 1st table + assertXPath(pLayout, "//tab[1]['pass 2']/infos/prtBounds", "left", u"1000"); + OUString width = getXPath(pLayout, "//tab[1]['pass 2']/row/cell[1]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(8000, width.toInt32(), 1); + width = getXPath(pLayout, "//tab[1]['pass 2']/row/cell[2]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(1000, width.toInt32(), 1); + // 2nd table + assertXPath(pLayout, "//tab[2]['pass 2']/infos/prtBounds", "left", u"1000"); + width = getXPath(pLayout, "//tab[2]['pass 2']/row/cell[1]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(7500, width.toInt32(), 1); + width = getXPath(pLayout, "//tab[2]['pass 2']/row/cell[2]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(1000, width.toInt32(), 1); + // 3rd table + assertXPath(pLayout, "//tab[3]['pass 2']/infos/prtBounds", "left", u"1000"); + width = getXPath(pLayout, "//tab[3]['pass 2']/row/cell[1]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(7000, width.toInt32(), 1); + width = getXPath(pLayout, "//tab[3]['pass 2']/row/cell[2]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(1000, width.toInt32(), 1); + // 4th table + assertXPath(pLayout, "//tab[4]['pass 2']/infos/prtBounds", "left", u"1000"); + width = getXPath(pLayout, "//tab[4]['pass 2']/row/cell[1]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(6500, width.toInt32(), 1); + width = getXPath(pLayout, "//tab[4]['pass 2']/row/cell[2]/infos/bounds", "width"); + CPPUNIT_ASSERT_DOUBLES_EQUAL(1000, width.toInt32(), 1); + } +} + } // end of anonymous namespace CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/writerfilter/rtftok/rtfdispatchsymbol.cxx b/sw/source/writerfilter/rtftok/rtfdispatchsymbol.cxx index 9a3ca31c127a..ba7c5c550154 100644 --- a/sw/source/writerfilter/rtftok/rtfdispatchsymbol.cxx +++ b/sw/source/writerfilter/rtftok/rtfdispatchsymbol.cxx @@ -220,7 +220,7 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword) m_aNestedTableCellsAttributes, m_nNestedCells)); prepareProperties(m_aStates.top(), pBuffer->GetParaProperties(), pBuffer->GetFrameProperties(), pBuffer->GetRowProperties(), - m_nNestedCells, m_nNestedCurrentCellX - m_nNestedTRLeft); + m_nNestedCells, m_nNestedCurrentCellX, m_nNestedTRLeft); if (m_aTableBufferStack.size() == 1 || !m_aStates.top().getCurrentBuffer()) { @@ -404,7 +404,7 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword) writerfilter::Reference<Properties>::Pointer_t frameProperties; writerfilter::Reference<Properties>::Pointer_t rowProperties; prepareProperties(m_aStates.top(), paraProperties, frameProperties, rowProperties, - m_nTopLevelCells, m_nTopLevelCurrentCellX - m_nTopLevelTRLeft); + m_nTopLevelCells, m_nTopLevelCurrentCellX, m_nTopLevelTRLeft); sendProperties(paraProperties, frameProperties, rowProperties); m_bNeedPap = true; diff --git a/sw/source/writerfilter/rtftok/rtfdispatchvalue.cxx b/sw/source/writerfilter/rtftok/rtfdispatchvalue.cxx index 3ec8542e644f..5b900f6e257e 100644 --- a/sw/source/writerfilter/rtftok/rtfdispatchvalue.cxx +++ b/sw/source/writerfilter/rtftok/rtfdispatchvalue.cxx @@ -386,32 +386,6 @@ bool RTFDocumentImpl::dispatchFrameValue(RTFKeyword nKeyword, int nParam) return false; } -static int GetCellWidth(int thisCellX, int prevCellX, RTFSprms& tableRowSprms) -{ - thisCellX -= prevCellX; - if (thisCellX == 0 && prevCellX > 0) - { - // If width of cell is 0, BUT there is a value for - // possible width. But if - // try to resolve this. - - // sw/source/filter/inc/wrtswtbl.hxx, minimal possible width of cells. - const int COL_DFLT_WIDTH = 41; - thisCellX = COL_DFLT_WIDTH; - } - // If there is a negative left margin, then the first cellx is relative to that. - if (prevCellX == 0) - { - if (RTFValue::Pointer_t pTblInd = tableRowSprms.find(NS_ooxml::LN_CT_TblPrBase_tblInd)) - { - RTFValue::Pointer_t pWidth = pTblInd->getAttributes().find(NS_ooxml::LN_CT_TblWidth_w); - if (pWidth && pWidth->getInt() < 0) - thisCellX = -1 * (pWidth->getInt() - prevCellX); - } - } - return thisCellX; -} - bool RTFDocumentImpl::dispatchTableValue(RTFKeyword nKeyword, int nParam) { int nSprm = 0; @@ -425,9 +399,9 @@ bool RTFDocumentImpl::dispatchTableValue(RTFKeyword nKeyword, int nParam) (Destination::NESTEDTABLEPROPERTIES == m_aStates.top().getDestination()) ? m_nNestedCurrentCellX : m_nTopLevelCurrentCellX); - int nCellX = GetCellWidth(nParam, rCurrentCellX, m_aStates.top().getTableRowSprms()); + int nCellWidth = nParam - rCurrentCellX; rCurrentCellX = nParam; - auto pXValue = new RTFValue(nCellX); + auto pXValue = new RTFValue(nCellWidth); m_aStates.top().getTableRowSprms().set(NS_ooxml::LN_CT_TblGridBase_gridCol, pXValue, RTFConflictPolicy::Append); if (Destination::NESTEDTABLEPROPERTIES == m_aStates.top().getDestination()) @@ -488,60 +462,18 @@ bool RTFDocumentImpl::dispatchTableValue(RTFKeyword nKeyword, int nParam) } break; case RTFKeyword::TRLEFT: - case RTFKeyword::TBLIND: { - // the value is in twips auto const aDestination = m_aStates.top().getDestination(); int& rCurrentTRLeft((Destination::NESTEDTABLEPROPERTIES == aDestination) ? m_nNestedTRLeft : m_nTopLevelTRLeft); - int& rCurrentCellX((Destination::NESTEDTABLEPROPERTIES == aDestination) - ? m_nNestedCurrentCellX - : m_nTopLevelCurrentCellX); - putNestedAttribute(m_aStates.top().getTableRowSprms(), NS_ooxml::LN_CT_TblPrBase_tblInd, - NS_ooxml::LN_CT_TblWidth_type, - new RTFValue(NS_ooxml::LN_Value_ST_TblWidth_dxa)); - - if (nKeyword == RTFKeyword::TBLIND) - { - RTFValue::Pointer_t pCellMargin - = m_aStates.top().getTableRowSprms().find(NS_ooxml::LN_CT_TblPrBase_tblCellMar); - if (pCellMargin) - { - RTFValue::Pointer_t pMarginLeft - = pCellMargin->getSprms().find(NS_ooxml::LN_CT_TcMar_left); - if (pMarginLeft) - nParam -= pMarginLeft->getAttributes() - .find(NS_ooxml::LN_CT_TblWidth_w) - ->getInt(); - } - } rCurrentTRLeft = nParam; - // Correct the first cellX, if already pushed before these. - // FIXME: this whole convoluted processing of CELLX, TRLEFT, TBLIND should be replaced - // with simple pushing of the respective values as is; and all that should eventually - // be processed in RTFKeyword::ROW handler (RTFDocumentImpl::dispatchSymbol), where all - // information would already be available. There we could know the table indent, row - // left offset, all right cell boundaries; and could calculate correct widths (likely - // in prepareProperties call). - bool hadCellX = false; - for (auto & [ id, pValue ] : m_aStates.top().getTableRowSprms()) - { - if (id == NS_ooxml::LN_CT_TblGridBase_gridCol) - { - if (int val = pValue->getInt(); val != -1) - { - val = GetCellWidth(val, nParam, m_aStates.top().getTableRowSprms()); - pValue = new RTFValue(val); - hadCellX = true; - break; - } - } - } - if (!hadCellX) - rCurrentCellX = rCurrentTRLeft; - putNestedAttribute(m_aStates.top().getTableRowSprms(), NS_ooxml::LN_CT_TblPrBase_tblInd, - +NS_ooxml::LN_CT_TblWidth_w, new RTFValue(nParam)); + return true; + } + break; + case RTFKeyword::TBLIND: + { + set_tblInd(m_aStates.top().getTableRowSprms(), nParam); return true; } break; diff --git a/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx b/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx index ba3b85c390ef..b7982553bc7c 100644 --- a/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx +++ b/sw/source/writerfilter/rtftok/rtfdocumentimpl.cxx @@ -1714,11 +1714,30 @@ void RTFDocumentImpl::text(OUString& rString) } } +void RTFDocumentImpl::set_tblInd(RTFSprms& tableRowSprms, int val) +{ + // the value is in twips + putNestedAttribute(tableRowSprms, NS_ooxml::LN_CT_TblPrBase_tblInd, + NS_ooxml::LN_CT_TblWidth_type, + new RTFValue(NS_ooxml::LN_Value_ST_TblWidth_dxa)); + + RTFValue::Pointer_t pCellMargin = tableRowSprms.find(NS_ooxml::LN_CT_TblPrBase_tblCellMar); + if (pCellMargin) + { + RTFValue::Pointer_t pMarginLeft = pCellMargin->getSprms().find(NS_ooxml::LN_CT_TcMar_left); + if (pMarginLeft) + val -= pMarginLeft->getAttributes().find(NS_ooxml::LN_CT_TblWidth_w)->getInt(); + } + + putNestedAttribute(tableRowSprms, NS_ooxml::LN_CT_TblPrBase_tblInd, +NS_ooxml::LN_CT_TblWidth_w, + new RTFValue(val)); +} + void RTFDocumentImpl::prepareProperties( RTFParserState& rState, writerfilter::Reference<Properties>::Pointer_t& o_rpParagraphProperties, writerfilter::Reference<Properties>::Pointer_t& o_rpFrameProperties, writerfilter::Reference<Properties>::Pointer_t& o_rpTableRowProperties, int const nCells, - int const nCurrentCellX) + int const nCurrentCellX, int nTRLeft) { o_rpParagraphProperties = getProperties(rState.getParagraphAttributes(), rState.getParagraphSprms(), @@ -1737,11 +1756,67 @@ void RTFDocumentImpl::prepareProperties( auto pUnitValue = new RTFValue(3); putNestedAttribute(rState.getTableRowSprms(), NS_ooxml::LN_CT_TblPrBase_tblW, NS_ooxml::LN_CT_TblWidth_type, pUnitValue); - auto pWValue = new RTFValue(nCurrentCellX); + auto pWValue = new RTFValue(nCurrentCellX - nTRLeft); putNestedAttribute(rState.getTableRowSprms(), NS_ooxml::LN_CT_TblPrBase_tblW, NS_ooxml::LN_CT_TblWidth_w, pWValue); } + // Correct cells' widths. + bool checkedMinusOne = false; + bool seenFirstColumn = false; + bool seenPositiveWidth = false; + for (auto & [ id, pValue ] : rState.getTableRowSprms()) + { + if (id == NS_ooxml::LN_CT_TblGridBase_gridCol) + { + int val = pValue->getInt(); + if (!checkedMinusOne) + { + // -1 is the special value set in RTFDocumentImpl::resetTableRowProperties + // and used in DomainMapperTableManager::sprm; skip it + checkedMinusOne = true; + if (val == -1) + continue; + } + if (!seenFirstColumn) + { + if (nTRLeft != 0) + { + // First cell: it was calculated against the initial value of *CurrentCellX, + // which is 0; now subtract nTRLeft from it + val -= nTRLeft; + pValue = new RTFValue(val); + } + seenFirstColumn = true; + if (val > 0) + seenPositiveWidth = true; + continue; + } + if (val > 0) + { + seenPositiveWidth = true; + continue; + } + // If width of this cell, and all previous cells, is 0, leave 0 so autofit will try + // to resolve this. But when there were proper widths before, use minimal width. + if (!seenPositiveWidth) + continue; + + // sw/source/filter/inc/wrtswtbl.hxx, minimal possible width of cells. + const int COL_DFLT_WIDTH = 41; + pValue = new RTFValue(COL_DFLT_WIDTH); + } + } + + 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)) + { + set_tblInd(rState.getTableRowSprms(), nTRLeft); + } + } + if (nCells > 0) rState.getTableRowSprms().set(NS_ooxml::LN_tblRow, new RTFValue(1)); diff --git a/sw/source/writerfilter/rtftok/rtfdocumentimpl.hxx b/sw/source/writerfilter/rtftok/rtfdocumentimpl.hxx index e3aa4c5ba558..c29a147157bd 100644 --- a/sw/source/writerfilter/rtftok/rtfdocumentimpl.hxx +++ b/sw/source/writerfilter/rtftok/rtfdocumentimpl.hxx @@ -792,11 +792,12 @@ private: void checkNeedPap(); void handleFontTableEntry(); void sectBreak(bool bFinal = false); + static void set_tblInd(RTFSprms& tableRowSprms, int val); void prepareProperties(RTFParserState& rState, writerfilter::Reference<Properties>::Pointer_t& o_rpParagraphProperties, writerfilter::Reference<Properties>::Pointer_t& o_rpFrameProperties, writerfilter::Reference<Properties>::Pointer_t& o_rpTableRowProperties, - int nCells, int nCurrentCellX); + int nCells, int nCurrentCellX, int nTRLeft); /// Send the passed properties to dmapper. void sendProperties(writerfilter::Reference<Properties>::Pointer_t const& pParagraphProperties, writerfilter::Reference<Properties>::Pointer_t const& pFrameProperties,
