Bug 20840 – Wrong collapsing border resolution when a table has cells with different colspans

• Component: Tables
• Status: NEW

[Michael Dayah]

In the linked site, the dark blue border wrapping the light blue border that wraps the Lanthanoids and Actinoids is drawn by the light blue border's cells. The cell containing the copyright notice and last modified date draws the darker blue border above and beneath it.

Clicking the Wide checkbox causes, first, cells 71 and 103 to be appended before 72 and 104. At the same time, the colspan of the copyright-containing cell is reduced with each iteration.

Strangely, two issues occur. The top border of the copyright cell does not reduce AND the bottom border is carried up with the appended cells, even though they have their own border which is not blue.

WebKit-r36309

[Michael Dayah]

Created attachment 26408 [details]
Test case

Webkit renders a border above all three cells. All other browsers only do above "abc"

[Michael Dayah]

In a border-collapsed table, when a cell shares a border with a colspanned cell, styling the single cell's border will style the entire colspanned cell's border, rather than partitioning it as happens when a cell shares a border with a colspanned cell and that single cell is not the farthest left.

[Oliver Hunt]

Confirmed -- Opera and Firefox disagree with our rendering

[Elliot Block]

Still repros latest Chrome5/Safari5 (which are at least WebKit 534 I believe).  Test case already attached looks good.

[Julien Chaffraix]

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

[Julien Chaffraix]

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

[Julien Chaffraix]

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

[Julien Chaffraix]

Created attachment 128649 [details]
Proposed change 1: Simple fix that should solve most of our colspan related bugs.

[Julien Chaffraix]

Created attachment 128785 [details]
Added Chromium linux+win baselines + corrected a too-eager-replace in test_expectations.txt

[Julien Chaffraix]

Created attachment 128799 [details]
Rebased patch to get EWS coverage.

[WebKit Review Bot]

Attachment 128799 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Traceback (most recent call last):
  File "Tools/Scripts/check-webkit-style", line 48, in <module>
    sys.exit(CheckWebKitStyle().main())
  File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/style/main.py", line 154, in main
    patch_checker.check(patch)
  File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/style/patchreader.py", line 66, in check
    self._text_file_reader.process_file(file_path=path, line_numbers=line_numbers)
  File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/style/filereader.py", line 130, in process_file
    self._processor.process(lines, file_path, **kwargs)
  File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/style/checker.py", line 838, in process
    checker.check(lines)
  File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/style/checkers/test_expectations.py", line 103, in check
    overrides = self._port_obj.test_expectations_overrides()
AttributeError: 'NoneType' object has no attribute 'test_expectations_overrides'


If any of these errors are false positives, please file a bug against check-webkit-style.

[Robert Hogan]

Comment on attachment 128799 [details]
Rebased patch to get EWS coverage.

View in context: https://bugs.webkit.org/attachment.cgi?id=128799&action=review

> Source/WebCore/rendering/RenderTableCell.cpp:571
> +bool RenderTableCell::considerCellForBeforeAfterCollapsingBorderResolution(const RenderTableCell* neightborCell) const

typo: should be neighbourCell

> Source/WebCore/rendering/RenderTableCell.cpp:587
> +    if (prevCell && considerCellForBeforeAfterCollapsingBorderResolution(prevCell)) {

maybe cellBorderAlignedWith() ? The function name does seem too long.

[Shezan Baig]

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

[Julien Chaffraix]

Comment on attachment 128799 [details]
Rebased patch to get EWS coverage.

View in context: https://bugs.webkit.org/attachment.cgi?id=128799&action=review

>> Source/WebCore/rendering/RenderTableCell.cpp:571
>> +bool RenderTableCell::considerCellForBeforeAfterCollapsingBorderResolution(const RenderTableCell* neightborCell) const
> 
> typo: should be neighbourCell

Good catch.

>> Source/WebCore/rendering/RenderTableCell.cpp:587
>> +    if (prevCell && considerCellForBeforeAfterCollapsingBorderResolution(prevCell)) {
> 
> maybe cellBorderAlignedWith() ? The function name does seem too long.

cellBorderAlignedWith is imprecise: we are only taking the block flow direction borders into account, not the inline base direction. Maybe areCellBeforeAfterBorderAlignedWith() or areCellBlockFlowDirectionAligned() would be better and a bit shorter.

[Ian Young]

This bug is clearly still an issue when compared to the "does what one expects" behavior in Gecko/Trident:

http://jsbin.com/efugat/1/edit

When the conflicting border-cell is to the left, the border-conflict draws the "winning" border across the whole spanned cell, but when it is on the right, the winning style is only drawn on the cell to which it was applied. 

This bug is also clearly the same as: 

* https://bugs.webkit.org/show_bug.cgi?id=5515
* https://bugs.webkit.org/show_bug.cgi?id=20260
* https://bugs.webkit.org/show_bug.cgi?id=109841

[Ahmad Saleem]

I am able to reproduce this issue in WebKit ToT (261901@main) using attached test case and also from Comment 15 JSBin, while Chrome Canary 113 and Firefox Nightly 113 match each other in both tests.