Bug 20840 - Wrong collapsing border resolution when a table has cells with different colspans
: Wrong collapsing border resolution when a table has cells with different cols...
Status: NEW
Product: WebKit
Classification: Unclassified
Component: Tables
: 528+ (Nightly build)
: PC Windows XP
: P2 Normal
Assigned To: Julien Chaffraix
http://www.dayah.com/periodic/
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-14 10:37 PDT by Michael Dayah
Modified: 2013-06-14 15:17 PDT (History)
15 users (show)

See Also:


Attachments
Test case (279 bytes, text/html)
2009-01-04 02:11 PST, Michael Dayah
no flags Details
Proposed change 1: Simple fix that should solve most of our colspan related bugs. (64.98 KB, patch)
2012-02-23 21:52 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Added Chromium linux+win baselines + corrected a too-eager-replace in test_expectations.txt (94.14 KB, patch)
2012-02-24 12:46 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Rebased patch to get EWS coverage. (93.99 KB, patch)
2012-02-24 13:42 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Dayah 2008-09-14 10:37:15 PDT
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
Comment 1 Michael Dayah 2009-01-04 02:11:33 PST
Created attachment 26408 [details]
Test case

Webkit renders a border above all three cells. All other browsers only do above "abc"
Comment 2 Michael Dayah 2009-01-04 02:24:01 PST
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.
Comment 3 Oliver Hunt 2009-01-04 02:56:46 PST
Confirmed -- Opera and Firefox disagree with our rendering
Comment 4 Elliot Block 2010-07-09 09:50:33 PDT
Still repros latest Chrome5/Safari5 (which are at least WebKit 534 I believe).  Test case already attached looks good.
Comment 5 Julien Chaffraix 2012-02-23 18:38:24 PST
*** Bug 23348 has been marked as a duplicate of this bug. ***
Comment 6 Julien Chaffraix 2012-02-23 18:38:41 PST
*** Bug 40286 has been marked as a duplicate of this bug. ***
Comment 7 Julien Chaffraix 2012-02-23 18:38:57 PST
*** Bug 42942 has been marked as a duplicate of this bug. ***
Comment 8 Julien Chaffraix 2012-02-23 21:52:18 PST
Created attachment 128649 [details]
Proposed change 1: Simple fix that should solve most of our colspan related bugs.
Comment 9 Julien Chaffraix 2012-02-24 12:46:12 PST
Created attachment 128785 [details]
Added Chromium linux+win baselines + corrected a too-eager-replace in test_expectations.txt
Comment 10 Julien Chaffraix 2012-02-24 13:42:55 PST
Created attachment 128799 [details]
Rebased patch to get EWS coverage.
Comment 11 WebKit Review Bot 2012-02-24 13:45:31 PST
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.
Comment 12 Robert Hogan 2012-02-29 00:57:23 PST
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.
Comment 13 Shezan Baig 2012-04-15 20:55:02 PDT
*** Bug 69723 has been marked as a duplicate of this bug. ***
Comment 14 Julien Chaffraix 2012-06-19 10:46:49 PDT
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.
Comment 15 Ian Young 2013-06-14 15:17:38 PDT
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