sw HTML export: fix invalid HTML when all cells of a row have the same rowspan

The bugdoc has a table with 2 columns and 2 rows, but both the A1 and
the A2 cell has rowspan=2, so there are only covered cells in the second
row. It seems there is no valid HTML markup to express this Writer doc
model.

What HTML seems to suggest instead is to simply decrease the amount of
rowspan attribute values and then the empty <tr> elements can be simply
skipped.

This fixes the

Row 2 of a row group established by a tbody element has no cells beginning on it.

from the w3c HTML validator.

Note that you can't create such problematic tables on the UI: the UI
will delete the row only containing covered cells for you when the last
cell gets merged. Such documents can be created by importers, though.

Change-Id: Ice4fa3636e8b780d374f3d319b198aaaada9f5e0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145402
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

2 files changed, 77 insertions, 8 deletions

diff --git a/sw/qa/filter/html/html.cxx b/sw/qa/filter/html/html.cxx
index 4ec815da0c56..537b437c4bdb 100644
--- a/sw/qa/filter/html/html.cxx
+++ b/sw/qa/filter/html/html.cxx
@@ -8,6 +8,7 @@
  */
 
 #include <swmodeltestbase.hxx>
+#include <test/htmltesttools.hxx>
 
 #include <vcl/gdimtf.hxx>
 
@@ -30,7 +31,7 @@ namespace
  * Keep using the various sw_<format>import/export suites for multiple filter calls inside a single
  * test.
  */
-class Test : public SwModelTestBase
+class Test : public SwModelTestBase, public HtmlTestTools
 {
 public:
     Test()
@@ -162,6 +163,39 @@ CPPUNIT_TEST_FIXTURE(Test, testTableCellFloatValueType)
     assertXPathNoAttribute(pXmlDoc, "//reqif-xhtml:td", "sdval");
     assertXPathNoAttribute(pXmlDoc, "//reqif-xhtml:td", "sdnum");
 }
+
+CPPUNIT_TEST_FIXTURE(Test, testTableRowSpanInAllCells)
+{
+    // Given a document with a 2x2 table, A1:A2 and B1:B2 is merged:
+    createSwDoc();
+    SwDoc* pDoc = getSwDoc();
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    SwInsertTableOptions aTableOptions(SwInsertTableFlags::DefaultBorder, 0);
+    pWrtShell->InsertTable(aTableOptions, /*nRows=*/2, /*nCols=*/2);
+    pWrtShell->MoveTable(GotoPrevTable, fnTableStart);
+    SwTableNode* pTableNode = pWrtShell->GetCursor()->GetPointNode().FindTableNode();
+    SwTable& rTable = pTableNode->GetTable();
+    auto pBox = const_cast<SwTableBox*>(rTable.GetTableBox("A1"));
+    pBox->setRowSpan(2);
+    pBox = const_cast<SwTableBox*>(rTable.GetTableBox("B1"));
+    pBox->setRowSpan(2);
+    pBox = const_cast<SwTableBox*>(rTable.GetTableBox("A2"));
+    pBox->setRowSpan(-1);
+    pBox = const_cast<SwTableBox*>(rTable.GetTableBox("B2"));
+    pBox->setRowSpan(-1);
+
+    // When exporting to HTML:
+    save("HTML (StarWriter)");
+
+    // Then make sure that the output is simplified to valid HTML, by omitting the rowspan attribute
+    // & the empty <tr> element:
+    htmlDocUniquePtr pHtmlDoc = parseHtml(maTempFile);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - XPath '//tr[1]/td[1]' unexpected 'rowspan' attribute
+    // i.e. a combination of rowspan + empty <tr> was emitted.
+    assertXPathNoAttribute(pHtmlDoc, "//tr[1]/td[1]", "rowspan");
+    assertXPath(pHtmlDoc, "//tr", 1);
+}
 }
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/source/filter/html/htmltabw.cxx b/sw/source/filter/html/htmltabw.cxx
index 43e9ebf97328..ddb8c8eb5f10 100644
--- a/sw/source/filter/html/htmltabw.cxx
+++ b/sw/source/filter/html/htmltabw.cxx
@@ -63,12 +63,20 @@ class SwHTMLWrtTable : public SwWriteTable
     static void Pixelize( sal_uInt16& rValue );
     void PixelizeBorders();
 
+    /// Writes a single table cell.
+    ///
+    /// bCellRowSpan decides if the cell's row span should be written or not.
     void OutTableCell( SwHTMLWriter& rWrt, const SwWriteTableCell *pCell,
-                       bool bOutVAlign ) const;
+                       bool bOutVAlign,
+                       bool bCellRowSpan ) const;
 
+    /// Writes a single table row.
+    ///
+    /// rSkipRows decides if the next N rows should be skipped or written.
     void OutTableCells( SwHTMLWriter& rWrt,
                         const SwWriteTableCells& rCells,
-                        const SvxBrushItem *pBrushItem ) const;
+                        const SvxBrushItem *pBrushItem,
+                        sal_uInt16& rSkipRows ) const;
 
     virtual bool ShouldExpandSub( const SwTableBox *pBox,
                             bool bExpandedBefore, sal_uInt16 nDepth ) const override;
@@ -254,7 +262,8 @@ bool SwHTMLWrtTable::ShouldExpandSub( const SwTableBox *pBox,
 // Write a box as single cell
 void SwHTMLWrtTable::OutTableCell( SwHTMLWriter& rWrt,
                                    const SwWriteTableCell *pCell,
-                                   bool bOutVAlign ) const
+                                   bool bOutVAlign,
+                                   bool bCellRowSpan ) const
 {
     const SwTableBox *pBox = pCell->GetBox();
     sal_uInt16 nRow = pCell->GetRow();
@@ -307,7 +316,7 @@ void SwHTMLWrtTable::OutTableCell( SwHTMLWriter& rWrt,
     sOut.append(rWrt.GetNamespace() + aTag);
 
     // output ROW- and COLSPAN
-    if( nRowSpan>1 )
+    if (nRowSpan > 1 && bCellRowSpan)
     {
         sOut.append(" " OOO_STRING_SVTOOLS_HTML_O_rowspan
                 "=\"" + OString::number(nRowSpan) + "\"");
@@ -505,7 +514,8 @@ void SwHTMLWrtTable::OutTableCell( SwHTMLWriter& rWrt,
 // output a line as lines
 void SwHTMLWrtTable::OutTableCells( SwHTMLWriter& rWrt,
                                     const SwWriteTableCells& rCells,
-                                    const SvxBrushItem *pBrushItem ) const
+                                    const SvxBrushItem *pBrushItem,
+                                    sal_uInt16& rSkipRows ) const
 {
     // If the line contains more the one cell and all cells have the same
     // alignment, then output the VALIGN at the line instead of the cell.
@@ -556,9 +566,26 @@ void SwHTMLWrtTable::OutTableCells( SwHTMLWriter& rWrt,
 
     rWrt.IncIndentLevel(); // indent content of <TR>...</TR>
 
+    bool bCellRowSpan = true;
+    if (!rCells.empty() && rCells[0]->GetRowSpan() > 1)
+    {
+        // Skip the rowspan attrs of <td> elements if they are the same for every cell of this row.
+        bCellRowSpan = std::adjacent_find(rCells.begin(), rCells.end(),
+                                          [](const std::unique_ptr<SwWriteTableCell>& pA,
+                                             const std::unique_ptr<SwWriteTableCell>& pB)
+                                          { return pA->GetRowSpan() != pB->GetRowSpan(); })
+                       != rCells.end();
+        if (!bCellRowSpan)
+        {
+            // If no rowspan is written, then skip rows which would only contain covered cells, but
+            // not the current row.
+            rSkipRows = rCells[0]->GetRowSpan() - 1;
+        }
+    }
+
     for (const auto &rpCell : rCells)
     {
-        OutTableCell(rWrt, rpCell.get(), text::VertOrientation::NONE == eRowVertOri);
+        OutTableCell(rWrt, rpCell.get(), text::VertOrientation::NONE == eRowVertOri, bCellRowSpan);
     }
 
     rWrt.DecIndentLevel(); // indent content of <TR>...</TR>
@@ -824,11 +851,19 @@ void SwHTMLWrtTable::Write( SwHTMLWriter& rWrt, sal_Int16 eAlign,
         rWrt.IncIndentLevel(); // indent content of <THEAD>/<TDATA>
     }
 
+    sal_uInt16 nSkipRows = 0;
     for( SwWriteTableRows::size_type nRow = 0; nRow < m_aRows.size(); ++nRow )
     {
         const SwWriteTableRow *pRow2 = m_aRows[nRow].get();
 
-        OutTableCells( rWrt, pRow2->GetCells(), pRow2->GetBackground() );
+        if (nSkipRows == 0)
+        {
+            OutTableCells(rWrt, pRow2->GetCells(), pRow2->GetBackground(), nSkipRows);
+        }
+        else
+        {
+            --nSkipRows;
+        }
         if( !m_nCellSpacing && nRow < m_aRows.size()-1 && pRow2->m_bBottomBorder &&
             pRow2->m_nBottomBorder > SvxBorderLineWidth::Thin )
         {