Bug 76553 – min-width is not implemented on <table> for table-layout: auto

• Component: Tables
• Status: RESOLVED FIXED

[Max Vujovic]

Created attachment 122950 [details]
Reproduction

I'm splitting this bug out from a master bug: Bug 12396 - min-width, max-width do not work on tables.

This bug will focus on the min-width case.
An existing bug already focuses on the max-width case: Bug 12460 - Max-width inheritance doesn't work

Currently, Firefox and Opera support min-width and max-width on the table element. However, IE9 and WebKit do not.

The CSS 2.1 spec states that min-width and max-width are undefined on tables. The exact line is: "In CSS 2.1, the effect of 'min-width' and 'max-width' on tables, inline tables, table cells, table columns, and column groups is undefined".

However, web developers want it, and Firefox and Opera treat min-width and max-width on tables in a consistent way that we can bring to WebKit. For a more detailed discussion, see this thread on the www-style mailing list: http://lists.w3.org/Archives/Public/www-style/2012Jan/0684.html

There is also a related Chromium bug: http://code.google.com/p/chromium/issues/detail?id=50169

In the attached reproduction, the table should be 800px wide because of the min-width property. It is 800px wide in Firefox and Opera, but 200px wide in WebKit.

[Max Vujovic]

Created attachment 122954 [details]
Patch

This patch adds support for min-width. I plan on discussing it with Julien.

[Philippe Normand]

Comment on attachment 122954 [details]
Patch

Attachment 122954 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11194288

[Max Vujovic]

Created attachment 122972 [details]
Patch

Attempting to resolve webkit-patch encoding error on GTK EWS.

[Dirk Schulze]

Comment on attachment 122972 [details]
Patch

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

This is not meant as a review, just some code comments.

> Source/WebCore/ChangeLog:7
> +        min-width does not work on <table>
> +        https://bugs.webkit.org/show_bug.cgi?id=76553
> +

Please go a bit more into detail when implementing new features. What did you implement, how did you implement it. For bigger patches a line by line comment next to the changed functions in the ChangeLog might help as well.

> Source/WebCore/rendering/RenderTable.cpp:273
> +    bool isCSSTable = !node() || !node()->hasTagName(tableTag);
> +    if (isCSSTable && (styleWidth.isFixed() && styleWidth.isPositive())) {

At first, no need to add a new boolean, just put it in the if clause. Can this renderer exist without its node? I don't think that this check is necessary. Do other elements than the HTMLTableElement access RenderTable as well? If not, you dan't need this check at all.

[Max Vujovic]

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

> > Source/WebCore/ChangeLog:7
> > +        min-width does not work on <table>
> > +        https://bugs.webkit.org/show_bug.cgi?id=76553
> > +
> 
> Please go a bit more into detail when implementing new features. What did you implement, how did you implement it. For bigger patches a line by line comment next to the changed functions in the ChangeLog might help as well.

Sure, will do.

> 
> > Source/WebCore/rendering/RenderTable.cpp:273
> > +    bool isCSSTable = !node() || !node()->hasTagName(tableTag);
> > +    if (isCSSTable && (styleWidth.isFixed() && styleWidth.isPositive())) {
> 
> At first, no need to add a new boolean, just put it in the if clause. Can this renderer exist without its node? I don't think that this check is necessary. Do other elements than the HTMLTableElement access RenderTable as well? If not, you dan't need this check at all.

This check addresses the case of an element besides table, such as a div, that has "display: table;" set. We're calling this a "CSS table," and a we're calling an actual table element an "HTML table". In both cases, a RenderTable will be created for the element. We need to differentiate between the cases because width styles on HTML tables include borders and padding, but width styles on CSS tables do not include borders and paddings, so we must include them in the width calculation.

As for the boolean, I think it makes the conditional expression easier to read, but I'll go ahead and remove it since based on our offline discussion, it does not conform to WebKit coding style.

[Max Vujovic]

Created attachment 123140 [details]
Patch

Updated patch: Removed isCSSTable boolean and added more comments to the ChangeLog.

[Julien Chaffraix]

Comment on attachment 123140 [details]
Patch

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

The change looks OK. Several questions and rough edges that I would like answered.

> LayoutTests/fast/table/script-tests/min-width.js:10
> +    table.setAttribute("style", "display: table; border-spacing: 0; background-color: #0f0; border: solid 0px #00f;" + extraTableStyle);

Shouldn't we consider the 'display: inline-table' case too?

> LayoutTests/fast/table/script-tests/min-width.js:121
> +shouldEvaluateTo("computeTableOffsetWidth(/*isCSSTable*/ true, /*min-intrinsic*/ 100, /*max-intrinsic*/ 250, /*extraTableStyle*/ 'min-width: auto;' + paddingsAndBorders30px)", 250+30);

You should have some testing for vertical writing-modes as your code seems to support this.

> Source/WebCore/ChangeLog:16
> +        * rendering/AutoTableLayout.cpp:
> +        (WebCore::AutoTableLayout::computePreferredLogicalWidths):
> +
> +            If the min or max preferred logical widths is less than a fixed min width style, it is
> +            set to the fixed min width style.

This only applies to automatic table layout which is only a subset of the goal of this bug. I am fine with repurposing the bug to support this subset. See the discussion on the www-style mailing list:

http://lists.w3.org/Archives/Public/www-style/2012Jan/0361.html

However we need to track the greater goal of also having fixed table layout on board.

> Source/WebCore/rendering/AutoTableLayout.cpp:268
> +    } else if (remainingPercent <= 0 && maxNonPercent) {

This change does not seem to be covered by your tests or am I missing something?

> Source/WebCore/rendering/AutoTableLayout.cpp:274
> +    if (tableLogicalMinWidth.isFixed() && tableLogicalMinWidth.value() > 0) {

Does other browser supports percent min-width? This is comment-worthy in any case as you did not mention that on the www-style mailing list.

> Source/WebCore/rendering/AutoTableLayout.cpp:276
> +        minWidth = max<int>(minWidth, tableLogicalMinWidth.value());
> +        maxWidth = max<int>(minWidth, maxWidth);

Should be max<LayoutUnit>

> Source/WebCore/rendering/RenderTable.cpp:232
> +    Length styleWidth = style()->logicalWidth();
> +    if ((styleWidth.isFixed() || styleWidth.isPercent()) && styleWidth.isPositive())
> +        setLogicalWidth(convertStyleWidthToComputedWidth(styleWidth, containerWidthInInlineDirection));        

s/styleWidth/styleLogicalWidth/g

> Source/WebCore/rendering/RenderTable.cpp:254
> +    Length styleMinWidth = style()->logicalMinWidth();
> +    if ((styleMinWidth.isFixed() || styleMinWidth.isPercent()) && styleMinWidth.isPositive())
> +        setLogicalWidth(max(logicalWidth(), convertStyleWidthToComputedWidth(styleMinWidth, availableLogicalWidth)));

s/styleMinWidth/styleMinLogicalWidth/g

> Source/WebCore/rendering/RenderTable.cpp:268
> +LayoutUnit RenderTable::convertStyleWidthToComputedWidth(const Length& styleWidth, LayoutUnit availableWidth)

s/styleWidth/styleLogicalWidth/g

> Source/WebCore/rendering/RenderTable.cpp:272
> +    if ((!node() || !node()->hasTagName(tableTag)) && styleWidth.isFixed() && styleWidth.isPositive()) {

While at it, it would be neat to add a boolean for part of the logic:

bool isCSSTable = !node() || !node()->hasTagName(tableTag);
if (isCSSTable && ...)

[Julien Chaffraix]

Comment on attachment 123140 [details]
Patch

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

>> Source/WebCore/rendering/AutoTableLayout.cpp:276
>> +        maxWidth = max<int>(minWidth, maxWidth);
> 
> Should be max<LayoutUnit>

Scratch that, it's me forgetting about table needing integer precision it was fine as it was. Emil or Levi, feel free to chime in if I missed something here.

[Levi Weintraub]

(In reply to comment #8)
> Scratch that, it's me forgetting about table needing integer precision it was fine as it was. Emil or Levi, feel free to chime in if I missed something here.

You're spot on. Integer precision is necessary for Tables, but good looking out!

[Max Vujovic]

Created attachment 123823 [details]
Patch

(In reply to comment #7)
> (From update of attachment 123140 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123140&action=review
> > LayoutTests/fast/table/script-tests/min-width.js:10
> > +    table.setAttribute("style", "display: table; border-spacing: 0; background-color: #0f0; border: solid 0px #00f;" + extraTableStyle);
> 
> Shouldn't we consider the 'display: inline-table' case too?

Added tests for display: inline-table.

> > LayoutTests/fast/table/script-tests/min-width.js:121
> > +shouldEvaluateTo("computeTableOffsetWidth(/*isCSSTable*/ true, /*min-intrinsic*/ 100, /*max-intrinsic*/ 250, /*extraTableStyle*/ 'min-width: auto;' + paddingsAndBorders30px)", 250+30);
> 
> You should have some testing for vertical writing-modes as your code seems to support this.

Added tests for writing-mode: vertical-rl and direction: rtl.

> 
> > Source/WebCore/ChangeLog:16
> > +        * rendering/AutoTableLayout.cpp:
> > +        (WebCore::AutoTableLayout::computePreferredLogicalWidths):
> > +
> > +            If the min or max preferred logical widths is less than a fixed min width style, it is
> > +            set to the fixed min width style.
> 
> This only applies to automatic table layout which is only a subset of the goal of this bug. I am fine with repurposing the bug to support this subset. See the discussion on the www-style mailing list:
> 
> http://lists.w3.org/Archives/Public/www-style/2012Jan/0361.html
> 
> However we need to track the greater goal of also having fixed table layout on board.
> 

Agreed. I'm repurposing and renaming this bug for the automatic table layout case of min-width. I've split out the fixed table layout min-width case in Bug 76948, and linked it to the common master bug 12396.

> > Source/WebCore/rendering/AutoTableLayout.cpp:268
> > +    } else if (remainingPercent <= 0 && maxNonPercent) {
> 
> This change does not seem to be covered by your tests or am I missing something?

I've removed this change from the patch because it's unrelated to implementing min-width. This change consisted of changing "!remainingPercent" to "remainingPercent <= 0" because remainingPercent is a float. I was worried that the logical negation operator (!) would not function correctly on the float if floating point error was present. However, I have not come up with a real example that causes this to happen yet, so I have not filed a bug.

For context, this check handles the case when percent width columns take up 100% of the table width, but there are also non-percent width columns that require some of the table's width. I've only seen this case described in a W3C editor's draft of table layout algorithms. Note that the draft warns that it is not being actively maintained:

(From http://dev.w3.org/csswg/css3-tables-algorithms/Overview.src.htm)
"c. When percent columns are demanding the full table width (100%) and there are also non-percent columns

Because the algorithm does not allow the sum of specified percentages to be larger than 100%, a special case might arise where the percent columns are demanding the full table width (100%) but there are also non-percent columns which require at least their minimum width. In this case, the tables preferred width could never be satisfied. The algorithm treats tables preferred width as infinitely large

tablePreferredWidth = INFINITE"

> > Source/WebCore/rendering/AutoTableLayout.cpp:274
> > +    if (tableLogicalMinWidth.isFixed() && tableLogicalMinWidth.value() > 0) {
> 
> Does other browser supports percent min-width? This is comment-worthy in any case as you did not mention that on the www-style mailing list.

Firefox and Opera support both fixed and percent min-width. This patch also supports both fixed and percent min-width. 

Percent min-width is handled in RenderTable::computeLogicalWidth, and is intentionally not used in AutoTableLayout's calculation of preferred min and max width. This is analogous to percent width, which is not used in the AutoTableLayout's calculation of the preferred widths. I've added a note about this in the ChangeLog. Here is the existing code in AutoTableLayout::computePreferredLogicalWidths which only looks at a fixed width, if present:

    if (tableLogicalWidth.isFixed() && tableLogicalWidth.value() > 0) {
        minWidth = max<int>(minWidth, tableLogicalWidth.value());
        maxWidth = minWidth;
    }

In the above code snippet, note that minWidth and maxWidth refer to the preferred min width and the preferred max width as calculated by AutoTableLayout, not the min-width and max-width styles.
    
The following line actually computes percent widths. This line is in the new method RenderTable::convertStyleLogicalWidthToComputedWidth:

    return styleLogicalWidth.calcMinValue(availableWidth) + borders;

> > Source/WebCore/rendering/RenderTable.cpp:232
> > +    Length styleWidth = style()->logicalWidth();
> > +    if ((styleWidth.isFixed() || styleWidth.isPercent()) && styleWidth.isPositive())
> > +        setLogicalWidth(convertStyleWidthToComputedWidth(styleWidth, containerWidthInInlineDirection));        
> 
> s/styleWidth/styleLogicalWidth/g

Done.

> > Source/WebCore/rendering/RenderTable.cpp:254
> > +    Length styleMinWidth = style()->logicalMinWidth();
> > +    if ((styleMinWidth.isFixed() || styleMinWidth.isPercent()) && styleMinWidth.isPositive())
> > +        setLogicalWidth(max(logicalWidth(), convertStyleWidthToComputedWidth(styleMinWidth, availableLogicalWidth)));
> 
> s/styleMinWidth/styleMinLogicalWidth/g

Done.

> > Source/WebCore/rendering/RenderTable.cpp:268
> > +LayoutUnit RenderTable::convertStyleWidthToComputedWidth(const Length& styleWidth, LayoutUnit availableWidth)
> 
> s/styleWidth/styleLogicalWidth/g

Done.

> > Source/WebCore/rendering/RenderTable.cpp:272
> > +    if ((!node() || !node()->hasTagName(tableTag)) && styleWidth.isFixed() && styleWidth.isPositive()) {
> 
> While at it, it would be neat to add a boolean for part of the logic:
> 
> bool isCSSTable = !node() || !node()->hasTagName(tableTag);
> if (isCSSTable && ...)

Done. I originally did this, but then took it out since Dirk didn't think it was appropriate because the boolean is only used once. If it's a matter of opinion rather than official WebKit coding style, I'd rather have the boolean because it's easier to read.

[Max Vujovic]

Comment on attachment 123823 [details]
Patch

Looks like I've got a conflict with this patch and some recent changes in RenderTable.cpp. I'll resolve it and reupload.

[Max Vujovic]

Created attachment 123850 [details]
Patch

Resolved conflicts and updated patch.

[Julien Chaffraix]

Comment on attachment 123850 [details]
Patch

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

> LayoutTests/fast/table/min-width-expected.txt:246
> +PASS successfullyParsed is true

The output is huge - which is a good thing as you are covering most of the cases - but I would love for the test to be split in 2 to help maintenance and review.

> LayoutTests/fast/table/script-tests/min-width.js:93
> +    var propertyValue = table[propertyName];

I don't think this is sound. I don't see where you store the style value on the object (I tried this way and I did not always get the right value). I may be missing something but it looks like you want the computed value here and not the raw CSS value. If that's the case, you should use window.getComputedStyle(table, null)[propertyName].

> Source/WebCore/ChangeLog:31
> +            In FixedTableLayout.cpp, the computePreferredLogicalWidths method looks like it has
> +            issues based on the comment: "FIXME: This entire calculation is incorrect for both
> +            minwidth and maxwidth." (minwidth and maxwidth refer to the preferred widths, not the
> +            min-width and max-width styles). I have not implemented min-width for FixedTableLayout
> +            in this patch since it requires some more research around that comment.
> +
> +            min-width for the table-layout: fixed case has been split out into this bug:
> +            https://bugs.webkit.org/show_bug.cgi?id=76948

This is a great comment, this is preferably put at the beginning of the CL to underline it.

> Source/WebCore/rendering/AutoTableLayout.cpp:274
> +    if (tableLogicalMinWidth.isFixed() && tableLogicalMinWidth.value() > 0) {

You can use Length::isPositive here.

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

styleLogicalWidth.isFixed() || styleLogicalWidth.isPercent() can be replaced by styleLogicalWidth.isSpecified()

> Source/WebCore/rendering/RenderTable.cpp:253
> +    if ((styleMinLogicalWidth.isFixed() || styleMinLogicalWidth.isPercent()) && styleMinLogicalWidth.isPositive())

Ditto here.

[Max Vujovic]

Created attachment 124402 [details]
Patch

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

Thanks for the review! I've upload a new patch in response to the comments.

> > LayoutTests/fast/table/min-width-expected.txt:246
> > +PASS successfullyParsed is true
> 
> The output is huge - which is a good thing as you are covering most of the cases - but I would love for the test to be split in 2 to help maintenance and review.

Agreed. I've decided to split the tests out into 4 to make it a little easier to manage:

min-width-html-table-block.js
min-width-html-table-inline.js
min-width-css-table-inline.js
min-width-css-table-block.js

They all use helper functions from a new JS file:

min-width-helpers.js

> > LayoutTests/fast/table/script-tests/min-width.js:93
> > +    var propertyValue = table[propertyName];
> 
> I don't think this is sound. I don't see where you store the style value on the object (I tried this way and I did not always get the right value). I may be missing something but it looks like you want the computed value here and not the raw CSS value. If that's the case, you should use window.getComputedStyle(table, null)[propertyName].

I like your approach better. You've got it right; I do want the computed value. Now, I'm querying the width and height properties instead of offsetWidth and offsetHeight, and I've changed that line to: 

    var propertyValue = window.getComputedStyle(table, null).getPropertyValue(propertyName);

I was querying table.offsetWidth through table["offsetWidth"], which seemed to work for my purposes. However, I do prefer getComputedStyle because it's in the DOM CSS spec and offsetWidth is not in any spec. According to Mozilla Developer Network, "offsetWidth is not part of any W3C specification or technical recommendation" (https://developer.mozilla.org/en/DOM/element.offsetWidth).

> > Source/WebCore/ChangeLog:31
> > +            In FixedTableLayout.cpp, the computePreferredLogicalWidths method looks like it has
> > +            issues based on the comment: "FIXME: This entire calculation is incorrect for both
> > +            minwidth and maxwidth." (minwidth and maxwidth refer to the preferred widths, not the
> > +            min-width and max-width styles). I have not implemented min-width for FixedTableLayout
> > +            in this patch since it requires some more research around that comment.
> > +
> > +            min-width for the table-layout: fixed case has been split out into this bug:
> > +            https://bugs.webkit.org/show_bug.cgi?id=76948
> 
> This is a great comment, this is preferably put at the beginning of the CL to underline it.

Sounds good. I've moved it up.

> > Source/WebCore/rendering/AutoTableLayout.cpp:274
> > +    if (tableLogicalMinWidth.isFixed() && tableLogicalMinWidth.value() > 0) {
> 
> You can use Length::isPositive here.

Done. I've also changed the existing code for width (not min-width) that appears just before this from:

    if (tableLogicalWidth.isFixed() && tableLogicalWidth > 0) {

To:

    if (tableLogicalWidth.isFixed() && tableLogicalWidth.isPositive()) {

> > Source/WebCore/rendering/RenderTable.cpp:231
> > +    if ((styleLogicalWidth.isFixed() || styleLogicalWidth.isPercent()) && styleLogicalWidth.isPositive())
> 
> styleLogicalWidth.isFixed() || styleLogicalWidth.isPercent() can be replaced by styleLogicalWidth.isSpecified()

Done. Nice catch! :).

> > Source/WebCore/rendering/RenderTable.cpp:253
> > +    if ((styleMinLogicalWidth.isFixed() || styleMinLogicalWidth.isPercent()) && styleMinLogicalWidth.isPositive())
> 
> Ditto here.

Done.

[Julien Chaffraix]

Comment on attachment 124402 [details]
Patch

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

I did not repeat the comment for all the tests but all comments apply to the 4 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, ...

> 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)

> 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.

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

Nit: we usually use <!DOCTYPE html>

[Max Vujovic]

Created attachment 124874 [details]
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!

[Julien Chaffraix]

Comment on attachment 124874 [details]
Patch

Thanks for beautifying the tests.

[Max Vujovic]

(In reply to comment #17)
> (From update of attachment 124874 [details])
> Thanks for beautifying the tests.

Of course. Thank you for all the help!

[WebKit Review Bot]

Comment on attachment 124874 [details]
Patch

Clearing flags on attachment: 124874

Committed r106479: <http://trac.webkit.org/changeset/106479>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.