[Webkit-unassigned] [Bug 76553] min-width is not implemented on <table> for table-layout: auto

[bugzilla-daemon at webkit.org]

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


Max Vujovic <mvujovic at adobe.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #124402|0                           |1
        is obsolete|                            |
 Attachment #124874|                            |review?, commit-queue?
               Flag|                            |




--- Comment #16 from Max Vujovic <mvujovic at adobe.com>  2012-01-31 20:14:40 PST ---
Created an attachment (id=124874)
 --> (https://bugs.webkit.org/attachment.cgi?id=124874&action=review)
Patch

(In reply to comment #15)
> (From update of attachment 124402 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124402&action=review

Thanks for the review! I've uploaded another patch to address the readability issues with the tests.

> > LayoutTests/fast/table/min-width-css-table-block-expected.txt:6
> > +PASS computeTableProperty(/*propertyName*/ 'width', /*isCSSTable*/ true, /*min-intrinsic*/ 100, /*max-intrinsic*/ 250, /*extraTableStyle*/ 'min-width: 500px; display: table; padding-left: 10px; padding-right: 10px; border-left-width: 5px; border-right-width: 5px; ') is '500px'
> 
> If you put all the default styles in a common CSS class and dumped it once at the beginning, the output would be way more readable! We know that a CSS table should use display: table, ...

I refactored the tests so that they produce much easier to read output but still describe what they are testing.

I took your suggestion and factored out the default styles into a new stylesheet: LayoutTests/fast/table/resources/min-width.css.

I tried displaying the style sheet at the beginning of each test file, but it didn't look good. Instead, I've put a note at the beginning of each test that links to the style sheet and describes the important properties that are constant across all of the tests in the file.

Here's the note:

"The stylesheet used to style the table in each test is available at: LayoutTests/fast/table/resources/min-width.css

Most importantly, note that each table has:
- minimum intrinsic width and height both equal to 100px based on the table content
- maximum intrinsic width and height both equal to 250px based on the table content
- borders and paddings that add up to 30px in both the horizontal and vertical directions
- a parent whose dimensions are 1000px by 1000px

The function signature of computeLogicalWidth is:
function computeLogicalWidth(writingMode, direction, tableStyle)"

> > LayoutTests/fast/table/min-width-css-table-block-expected.txt:11
> > +PASS computeTableProperty(/*propertyName*/ 'width', /*isCSSTable*/ true, /*min-intrinsic*/ 100, /*max-intrinsic*/ 250, /*extraTableStyle*/ 'min-width: 500px; width: 60%; display: table; padding-left: 10px; padding-right: 10px; border-left-width: 5px; border-right-width: 5px; ') is '570px'
> 
> Interestingly we don't include borders & padding if width is a percentage for CSS table. This is inconsistent with the HTML case or even the CSS one. Is this what other browsers return? (agreed, it's another issue but it could be bug-worthy)

I opened bug 77028 a little while ago about this issue. Firefox and Opera display borders and paddings in addition to percent width. WebKit does not. WebKit considers borders and paddings as already included in the percent width. After bug 77028 gets fixed, I can update the expected test results for percent min-width.

> > LayoutTests/fast/table/min-width-css-table-block-expected.txt:36
> > +PASS computeTableProperty(/*propertyName*/ 'height', /*isCSSTable*/ true, /*min-intrinsic*/ 100, /*max-intrinsic*/ 250, /*extraTableStyle*/ 'min-height: 500px; display: table; -webkit-writing-mode: vertical-rl; writing-mode: vertical-rl; padding-top: 10px; padding-bottom: 10px; border-top-width: 5px; border-bottom-width: 5px; ') is '500px'
> 
> Please add a debug() comment here and an empty line to see the switch between writing modes.

Done. I've added debug() comments to highlight the switches between writing modes and directions. Thanks for the tip! It looks much better now.

> > LayoutTests/fast/table/min-width-css-table-block.html:1
> > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
> 
> Nit: we usually use <!DOCTYPE html>

Done. Good to know!

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