[Webkit-unassigned] [Bug 74955] WebKit adds vertical paddings and borders to the fixed width of CSS tables

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Julien Chaffraix <jchaffraix at webkit.org>  2011-12-21 14:29:25 PST ---
(From update of attachment 120087)
View in context: https://bugs.webkit.org/attachment.cgi?id=120087&action=review

Good catch! Some comments but the change is very fine.

> LayoutTests/fast/table/script-tests/css-table-width.js:3
> +description(
> +"This test checks that the width style is applied correctly to CSS tables with respect to table paddings and borders."
> +);

You can put that on one line.

> LayoutTests/fast/table/script-tests/css-table-width.js:40
> +shouldEvaluateTo("computeCSSTableWidth('width: 200px; border-style: solid; border-width: 10px 2px 30px 4px; padding: 50px 6px 70px 8px;')", 200+2+4+6+8);
> +shouldEvaluateTo("computeCSSTableWidth('width: 200px; border-style: solid; border-width: 10px 2px 30px 4px; padding: 50px 6px 70px 8px; border-collapse: collapse;')", 200 + (2+4)/2);

I would love to see some examples with a vertical writing mode (like 'writing-mode: vertical-rl') and some values of 'text-orientation' / 'direction' as this test feels not sufficient. Also a vertical writing mode would have shown the issue pretty easily (just put some padding in the inline base direction but not in the block flow direction).

> Source/WebCore/rendering/RenderTable.cpp:233
> +        LayoutUnit bordersAndPadding = 0;

I am not too fond of the naming. You don't include the paddings if you are on a collapsing border CSS table.

I couldn't find anything better (my take was bordersAndPaddingsOnNonCollapsingBordersTable which is too long). The variable name should at least be bordersAndPaddings for consistency.

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