Bug 107576 – REGRESION(r130774): preferred width of tables does not take max-width into account

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Tony Chang]

We forgot to update computePreferredLogicalWidth to consider max-width when adding this to tables in bug 98455.  This causes us to have lots of extra space when we shrink wrap a table with max-width.

Ojan volunteered to put together a fix that we can merge to the release and beta branches.

Originally reported in https://code.google.com/p/chromium/issues/detail?id=170642 .

[Ojan Vafai]

Created attachment 184042 [details]
Patch

[Julien Chaffraix]

Comment on attachment 184042 [details]
Patch

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

> Source/WebCore/rendering/RenderTable.cpp:730
> +    // FIXME: This should probably be checking for isSpecified since you should be able to use percentage, calc or viewport relative values for min-width.

Why don't we fix that now instead of waiting?

> LayoutTests/ChangeLog:9
> +        * fast/table/min-max-width-preferred-size.html: Added.

It would be nice to have some testing with box-sizing set on the table.

> LayoutTests/fast/table/min-max-width-preferred-size.html:1
> +<style>

Do we really need a quirksmode test?

[WebKit Review Bot]

Comment on attachment 184042 [details]
Patch

Clearing flags on attachment: 184042

Committed r140479: <http://trac.webkit.org/changeset/140479>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ojan Vafai]

Comment on attachment 184042 [details]
Patch

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

>> Source/WebCore/rendering/RenderTable.cpp:730
>> +    // FIXME: This should probably be checking for isSpecified since you should be able to use percentage, calc or viewport relative values for min-width.
> 
> Why don't we fix that now instead of waiting?

This is done this way throughout the rendering code. My plan was to get rid of all the copy-paste first and then fix this in the couple of places that remain.

>> LayoutTests/ChangeLog:9
>> +        * fast/table/min-max-width-preferred-size.html: Added.
> 
> It would be nice to have some testing with box-sizing set on the table.

OK. I'll add this in a followup patch.

>> LayoutTests/fast/table/min-max-width-preferred-size.html:1
>> +<style>
> 
> Do we really need a quirksmode test?

Whoops. That was just a mistake.