Bug 185341 – rowspan="0" results in different table layout than Firefox/Chrome

• Status: RESOLVED
• Resolution: FIXED
• Component: Tables

[Simon Fraser (smfr)]

Created attachment 339629 [details]
Testcase

Attached test case lays out differently to Chrome and Firefox; triggers by the rowspan="0"

[Ahmad Saleem]

Created attachment 461698 [details]
Safari 156 and STP 151 differs from other browsers

I am able to reproduce this bug in Safari 15.6.1 and Safari TP 151 using attached test case as can be shown in the screenshot and it differs from other browsers. Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/133910430>

[Karl Dubost]

https://searchfox.org/wubkat/rev/36d40c7dfd972e688c9c0febea6821825a85d6ac/Source/WebCore/html/HTMLTableCellElement.cpp#60-64
```cpp
unsigned HTMLTableCellElement::rowSpan() const
{
    // FIXME: a rowSpan equal to 0 should be allowed, and mean that the cell is to span all the remaining rows in the row group.
    return std::max(1u, rowSpanForBindings());
}
```

[Karl Dubost]

https://html.spec.whatwg.org/multipage/tables.html#attr-tdth-rowspan
> The td and th elements may also have a rowspan content attribute specified, 
> whose value must be 
> a valid non-negative integer less than or equal to 65534. 
> For this attribute, the value zero means that the cell is to span all the remaining rows in the row group.

Basically is the equivalent of an imaginary all keyword.

> The rowSpan IDL attribute must reflect the rowspan content attribute. 
> It is clamped to the range [0, 65534], 
> and its default value is 1.

[Karl Dubost]

It will have consequences into 

https://searchfox.org/wubkat/rev/36d40c7dfd972e688c9c0febea6821825a85d6ac/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp#273-283
```cpp
if (is<RenderTableCell>(renderer)) {
    auto* tableCellElement = renderer.element();
    if (auto* cellElement = dynamicDowncast<HTMLTableCellElement>(tableCellElement)) {
        auto rowSpan = cellElement->rowSpan();
        if (rowSpan > 1)
            childLayoutBox->setRowSpan(rowSpan);
        auto columnSpan = cellElement->colSpan();
        if (columnSpan > 1)
            childLayoutBox->setColumnSpan(columnSpan);
    }
}
```

and also into


https://searchfox.org/wubkat/rev/36d40c7dfd972e688c9c0febea6821825a85d6ac/Source/WebCore/rendering/RenderTableCell.cpp#127-138
```cpp
unsigned RenderTableCell::parseRowSpanFromDOM() const
{
    ASSERT(element());
    if (auto* cell = dynamicDowncast<HTMLTableCellElement>(*element()))
        return std::min<unsigned>(cell->rowSpan(), maxRowIndex);
#if ENABLE(MATHML)
    auto* mathMLElement = dynamicDowncast<MathMLElement>(*element());
    if (mathMLElement && mathMLElement->hasTagName(MathMLNames::mtdTag))
        return std::min<unsigned>(mathMLElement->rowSpan(), maxRowIndex);
#endif
    return 1;
}
```

[Karl Dubost]

Created attachment 473677 [details]
rendering in safari, firefox, chrome (codepen)

So the rendering is messy in any browsers and not totally logical. 
But that's interesting to see how this is working.

The rowspan should be an easy fix. 

After there are stylistic differences on how borders are rendered in different circumstances.

[Karl Dubost]

Created attachment 473682 [details]
rendering in safari, minibrowser, firefox, chrome

I have a patch to fix it. But let's try a couple of tests.

[Karl Dubost]

I need to create some local tests to see if I can break my patch with weird testcases.

[Karl Dubost]

Pull request: https://github.com/WebKit/WebKit/pull/38398

[EWS]

Committed 288746@main (75a5507d4d8f): <https://commits.webkit.org/288746@main>

Reviewed commits have been landed. Closing PR #38398 and removing active labels.

[Karl Dubost]

*** Bug 255853 has been marked as a duplicate of this bug. ***