[webkit-reviews] review canceled: [Bug 77028] percent width includes borders and paddings on CSS tables : [Attachment 171381] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has canceled Arpita Bahuguna
<arpitabahuguna at gmail.com>'s request for review:
Bug 77028: percent width includes borders and paddings on CSS tables
https://bugs.webkit.org/show_bug.cgi?id=77028

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

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


> Source/WebCore/rendering/RenderTable.cpp:321
> +    if (isCSSTable && (styleLogicalWidth.isFixed() ||
styleLogicalWidth.isPercent()) && styleLogicalWidth.isPositive()) {

Shouldn't *any* specified width be handled in this way? This will fix the issue
for percent but other values still have the issue (e.g. viewport relative
length).

We should fix all cases at once by using styleLogicalWidth.isSpecified()
instead of just patching our way around.

> LayoutTests/fast/table/css-table-width-with-border-padding.html:43
> +    </div>

This should probably be a check-layout.js (the upside is that it would run
faster).

> LayoutTests/fast/table/css-table-width-with-border-padding.html:46
> +</html>

It would be nice to have some checks against box-sizing values as part of this
too. That makes sure we don't break it (which I don't think you do).