[Webkit-unassigned] [Bug 76246] Incorrect rendering of borders on <col> with span > 1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 12 18:42:55 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=76246


Julien Chaffraix <jchaffraix at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #161966|review?                     |
               Flag|                            |




--- Comment #6 from Julien Chaffraix <jchaffraix at webkit.org>  2012-09-12 18:43:20 PST ---
(From update of attachment 161966)
View in context: https://bugs.webkit.org/attachment.cgi?id=161966&action=review

> Source/WebCore/rendering/RenderTableCell.cpp:458
> +            // Next, irrespective of whether it is a spanned col or not, we apply its start border.

It would be nice to quote HTML5 here to know why we apply the col every time.

> Source/WebCore/rendering/RenderTableCell.cpp:459
> +            result = chooseBorder(result, CollapsedBorderValue(colElt->style()->borderStart(), includeColor ? colElt->style()->visitedDependentColor(startColorProperty) : Color(), BCOL));

The order in which you apply the borders here is the opposite of what CSS uses (column should win over column-group per 17.6.2.1 point 4). You should apply |colElt| first and then find its enclosingColumnGroup to apply the border. This should also be tested.

> Source/WebCore/rendering/RenderTableCell.cpp:475
> +                result = chooseBorder(CollapsedBorderValue(colElt->style()->borderEnd(), includeColor ? colElt->style()->visitedDependentColor(endColorProperty) : Color(), BCOL), result);

result should be first as we return the first parameter in case of equivalent borders. The code is not really consistent here unfortunately and we should try to avoid adding more latent bugs. It should be easily testable using equivalent borders of different color.

> Source/WebCore/rendering/RenderTableCell.cpp:573
> +                if (RenderTableCol* enclosingColumnGroup = colElt->enclosingColumnGroupIfAdjacentBefore()) {

Same remark about the ordering here too.

> LayoutTests/ChangeLog:11
> +        Modified the result of the existing test case which consists of a table with
> +        col and colgroup specified.

I have a hard time understand what changed. None of the borders have changed, yet your element is bigger. Could you explain what the change is and why it occurs?

> LayoutTests/ChangeLog:15
> +        * fast/table/border-collapsing/collapsed-border-with-col-colgroup-span-expected.png: Added.
> +        * fast/table/border-collapsing/collapsed-border-with-col-colgroup-span-expected.txt: Added.
> +        * fast/table/border-collapsing/collapsed-border-with-col-colgroup-span.html: Added.

Could it be possible to change this into a ref-test?

> LayoutTests/fast/table/border-collapsing/collapsed-border-with-col-colgroup-span.html:26
> +<col span=3 style="border: 1px solid red;"></col>

red should be reserved for failure.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list