[webkit-reviews] review denied: [Bug 5515] Border collapse problem with rowspan/colspan cells : [Attachment 159860] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has denied Arpita Bahuguna
<arpitabahuguna at gmail.com>'s request for review:
Bug 5515: Border collapse problem with rowspan/colspan cells
https://bugs.webkit.org/show_bug.cgi?id=5515

Attachment 159860: Patch
https://bugs.webkit.org/attachment.cgi?id=159860&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=159860&action=review


> Source/WebCore/rendering/RenderTableCell.cpp:498
> +    if (!isEndColumn && !(rowSpan() > 1)) {

This fix is not right. First you are only touching half of the collapsing
border functions which means that it is still broken in half of the cases.
Secondly, after this change, you are ignoring adjacent rowSpan / colspan > 1
cells that still have a full common border which I think is unfortunate. The
spec is silent on what we are supposed to do with rowspan / colspan but it
doesn't make much sense to disallow this case, especially since we handle it
properly already and other browsers seems to allow it too. I posted something
similar some time ago on bug 20840 (only for the colspan case) but never
followed up on it due to lack of time.

Note that your fix is a hack around the fact that we should support border
segmenting per bug 20260 and you should say it, not hide this fact. Adding this
would be the best but it would make our collapsing border code more complex and
we would probably run this change through the standards for inter-operability.