Bug 69723 – Visual glitch in webkit html table respect both firefox and IE

• Component: Tables
• Status: RESOLVED DUPLICATE

[Nelson Benitez]

Steps to reproduce the problem:
1. Download the webkitbug.html and tools24.png attachment files and put on same folder.
2. Open webkitbug.html file in chrome, firefox and IE browser

Firefox and IE displays fine, you can find how it should look in attachment GOOD_FIREFOX.png

Webkit seems to made a mess with two last rows of the table (which are hidden) and shows an inappropiate extra border in the second column, you can see that in attachment BAD_CHROME.png .

[Nelson Benitez]

Created attachment 110300 [details]
html test case

[Nelson Benitez]

Created attachment 110301 [details]
Image for html testcase

[Nelson Benitez]

Created attachment 110302 [details]
Looking good in firefox and IE

[Nelson Benitez]

Created attachment 110303 [details]
Looking bad in Webkit

[Shezan Baig]

Created attachment 137149 [details]
Patch

[Julien Chaffraix]

Comment on attachment 137149 [details]
Patch

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

This bug is a duplicate of bug 20840 for which I posted a patch some time ago but never followed up. Please check for duplicates when fixing bugs as this is a super old bug.

> Source/WebCore/rendering/RenderTableCell.cpp:417
> +bool RenderTableCell::isHorizontallyWithin(const RenderTableCell* other) const
> +{
> +    return col() >= other->col()
> +        && col() + colSpan() <= other->col() + other->colSpan();
> +}
> +
> +bool RenderTableCell::isVerticallyWithin(const RenderTableCell* other) const
> +{
> +    return row() >= other->row()
> +        && row() + rowSpan() <= other->row() + other->rowSpan();
> +}

I don't really think that matches other browsers. Also I really think it's not a good change: it completely ignores the fact that the 2 cells can be in any configurations (above / under, left / right) and thus only checks for some of the cases.

It's also not specified in CSS 2.1 so we should keep our behavior simple and not tied to a complex algorithm. The best way would be to get this behavior specified (likely matching FF, which means resolving collapsed borders by comparing adjacent cells even if that means cutting the border for colspan or rowspan) and implement it properly.

> LayoutTests/fast/table/border-collapsing/col-row-span-collapsing-expected.txt:1
> +CONSOLE MESSAGE: line 67: TypeError: 'undefined' is not an object (evaluating 'divs[i].style.display = "none"')

We prefer tests that don't have exceptions.

[Shezan Baig]

Thanks for taking a look, Julien :)

(In reply to comment #6)
> This bug is a duplicate of bug 20840 for which I posted a patch some time ago 
> but never followed up. Please check for duplicates when fixing bugs as this 
> is a super old bug.

Sorry about that, I'll mark this bug as a duplicate of bug 20840



> > Source/WebCore/rendering/RenderTableCell.cpp:417
> > +bool RenderTableCell::isHorizontallyWithin(const RenderTableCell* other) const
> > +{
> > +    return col() >= other->col()
> > +        && col() + colSpan() <= other->col() + other->colSpan();
> > +}
> > +
> > +bool RenderTableCell::isVerticallyWithin(const RenderTableCell* other) const
> > +{
> > +    return row() >= other->row()
> > +        && row() + rowSpan() <= other->row() + other->rowSpan();
> > +}
> 
> I don't really think that matches other browsers. Also I really think it's 
> not a good change: it completely ignores the fact that the 2 cells can be in 
> any configurations (above / under, left / right) and thus only checks for 
> some of the cases.


I'm not sure what exactly you mean here, because it seems to me like the above/under/left/right cases are handled by the changes in computeCollapsedBeforeBorder, computeCollapsedAfterBorder, computeCollapsedStartBorder, computeCollapsedEndBorder respectively.  Unless there's something else I didn't consider?


> It's also not specified in CSS 2.1 so we should keep our behavior simple and 
> not tied to a complex algorithm. The best way would be to get this behavior 
> specified (likely matching FF, which means resolving collapsed borders by 
> comparing adjacent cells even if that means cutting the border for colspan or 
> rowspan) and implement it properly.

Sounds like a good plan.


> > LayoutTests/fast/table/border-collapsing/col-row-span-collapsing-expected.txt:1
> > +CONSOLE MESSAGE: line 67: TypeError: 'undefined' is not an object (evaluating 'divs[i].style.display = "none"')
> 
> We prefer tests that don't have exceptions.


Hmm sorry about that.  I know why the exception could happen, but somehow it didn't happen when I ran the tests locally.

[Shezan Baig]

*** This bug has been marked as a duplicate of bug 20840 ***