Bug 311117 – Potential bug in table border code caught in code review

• Component: Tables
• Status: RESOLVED FIXED

[Sam Weinig]

I was working on a separate border issue and I noticed a weirdness in RenderTable::outerBorderTop().

```
inline LayoutUnit RenderTable::outerBorderTop() const
{
    if (writingMode().isHorizontal())
        return writingMode().isBlockTopToBottom() ? outerBorderBefore() : outerBorderAfter();
    return writingMode().isInlineTopToBottom() ? outerBorderStart() : borderEnd();
}
```

Notice that it uses `borderEnd()` for the last conditional, not `outerBorderEnd()`.

All the other variants of this function only use "outerBorder*()` calls. For instance, `RenderTable::outerBorderRight()`:

```
inline LayoutUnit RenderTable::outerBorderRight() const
{
    if (writingMode().isHorizontal())
        return writingMode().isInlineLeftToRight() ? outerBorderEnd() : outerBorderStart();
    return writingMode().isBlockLeftToRight() ? outerBorderAfter() : outerBorderBefore();
}
```

This looks like an accidental typo (otherwise, I would expect some kind of comment explaining the inconsistency in the pattern).

I haven't tried to figure out how to trigger this code or make a test case, but wanted to file the bug so I or someone else might take a look in the future.

[Karl Dubost]

This was changed in 
https://searchfox.org/wubkat/diff/72e07604aff9e355a3b669b908981bb66e8e597a/Source/WebCore/rendering/RenderTableInlines.h#96
It had the right term before. 

I didn't manage to create a test offering a visual difference. 
Plus there are other bugs getting into the way. 


table {
    writing-mode: vertical-rl;
    direction: rtl;
    border-collapse: collapse;
    border: none;
}
td {
    width: 30px;
    height: 30px;
}
/* First row: thin border (what borderEnd sees) */
.thin td {
    border: 2px solid blue;
}
/* Second row: thick border (what outerBorderEnd sees) */
.thick td {
    border: 40px solid red;
}

<table>
    <tbody>
        <tr class="thin">
            <td></td>
            <td></td>
        </tr>
        <tr class="thick">
            <td></td>
            <td></td>
        </tr>
    </tbody>
</table>



The table must have `border: none`

If the table has a visible border (e.g. `border: 2px solid black`), then `outerBorderEnd()` returns early at line 1536-1537 with the table's own border value — it never reaches the section iteration loop. Both `borderEnd()` and `outerBorderEnd()` would return the same value, and the bug is invisible.

https://searchfox.org/wubkat/rev/9234d58780c9dd9eaf6c8ad43dac914866768956/Source/WebCore/rendering/RenderTable.cpp#1536-1537
```cpp
if (tb.hasVisibleStyle())
    return CollapsedBorderValue::adjustedCollapsedBorderWidth(Style::evaluate<float>(tb.width, Style::ZoomNeeded { }), protect(document())->deviceScaleFactor(), !writingMode().isInlineFlipped());
```

With `border: none`, `outerBorderEnd()` falls through to the loop (line 1539-1546)
that iterates all sections and finds the thick border from row 2.

https://searchfox.org/wubkat/rev/9234d58780c9dd9eaf6c8ad43dac914866768956/Source/WebCore/rendering/RenderTable.cpp#1539-1546
```cpp
bool allHidden = true;
for (RenderTableSection* section = topSection(); section; section = sectionBelow(section)) {
    LayoutUnit sw = section->outerBorderEnd();
    if (sw < 0)
        continue;
    allHidden = false;
    borderWidth = std::max(borderWidth, sw);
}
```

But the bug has no really noticeable visual effect.

[Radar WebKit Bug Importer]

<rdar://problem/173742600>

[Karl Dubost]

CSS 2.2 §17.6.2 
https://www.w3.org/TR/CSS22/tables.html#collapsing-borders
  - "initial left and right border width" from first row only
  - "subsequent rows" with larger borders spilling into margin
  - "Any borders that spill into the margin are taken into account when determining if the table overflows some ancestor"

CSS Tables Level 3:
https://drafts.csswg.org/css-tables-3/#computing-cell-measures
  - Defines "table intrinsic offsets" (§3.8.2) as "half the winning border-width" for collapsed-borders mode
  - Has an open issue about this: "Handling of intrinsic offsets when in border collapsing mode" (https://github.com/w3c/csswg-drafts/issues/608)
  - Does not replicate the CSS 2.2 "first row" vs "all rows" distinction or the "spill into margin" overflow language
  - Does not mention ink/visual/scrollable overflow for collapsed borders at all

there is an open issue (#608) specifically about collapsed border intrinsic offsets. 
https://github.com/w3c/csswg-drafts/issues/608

Now I wonder if it was intentional from Elika.

[Karl Dubost]

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

[EWS]

Committed 310404@main (6885c2bcb70b): <https://commits.webkit.org/310404@main>

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

[Karl Dubost]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/58929