Bug 115156 – Fix table sizing when 'max-width' is used

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ryosuke Niwa]

https://chromium.googlesource.com/chromium/blink/+/0bca0dec4895aeeb2054ba36316e984e4ebed06f
Fix table sizing when 'max-width' is used

r143534 make <table> abide by 'max-width' all the time which is wrong.
Per the CSS specification, a table should be wide enough to fit its
content, regardless of 'max-width'.

r140479 fixed part of the regression from that change but made the
same fatal mistake by constraining min-content to fit 'max-width'.

The fix is to avoid constraining min-content and ensure that the table
logical width is at least its min-content size.

[László Langó]

Created attachment 218857 [details]
Patch

[László Langó]

Can anyone review this?

[Andreas Kling]

Comment on attachment 218857 [details]
Patch

OK

[WebKit Commit Bot]

Comment on attachment 218857 [details]
Patch

Clearing flags on attachment: 218857

Committed r163165: <http://trac.webkit.org/changeset/163165>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This change made two regression tests assert:

fast/table/multiple-captions-crash.xhtml
fast/table/multiple-captions-crash2.xhtml

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000105bf7daa WTFCrash + 42 (Assertions.cpp:333)
1   com.apple.WebCore             	0x0000000108ae4712 WebCore::RenderTable::updateLogicalWidth() + 3618 (RenderTable.cpp:334)
2   com.apple.WebCore             	0x0000000108ae53c9 WebCore::RenderTable::layout() + 569 (RenderTable.cpp:437)
3   com.apple.WebCore             	0x0000000109148ebb WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) + 1323 (RenderBlockFlow.cpp:640)
4   com.apple.WebCore             	0x0000000109146ada WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) + 618 (RenderBlockFlow.cpp:557)
5   com.apple.WebCore             	0x00000001091456b7 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) + 1191 (RenderBlockFlow.cpp:407)
6   com.apple.WebCore             	0x000000010887c55d WebCore::RenderBlock::layout() + 125 (RenderBlock.cpp:1318)
7   com.apple.WebCore             	0x0000000109148ebb WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) + 1323 (RenderBlockFlow.cpp:640)
8   com.apple.WebCore             	0x0000000109146ada WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) + 618 (RenderBlockFlow.cpp:557)

[WebKit Commit Bot]

Re-opened since this is blocked by bug 127997

[László Langó]

I cannot reproduce the assertions.

(In reply to comment #6)
> This change made two regression tests assert:
> 
> fast/table/multiple-captions-crash.xhtml
> fast/table/multiple-captions-crash2.xhtml
> 
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0   com.apple.JavaScriptCore          0x0000000105bf7daa WTFCrash + 42 (Assertions.cpp:333)
> 1   com.apple.WebCore                 0x0000000108ae4712 WebCore::RenderTable::updateLogicalWidth() + 3618 (RenderTable.cpp:334)
> 2   com.apple.WebCore                 0x0000000108ae53c9 WebCore::RenderTable::layout() + 569 (RenderTable.cpp:437)
> 3   com.apple.WebCore                 0x0000000109148ebb WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) + 1323 (RenderBlockFlow.cpp:640)
> 4   com.apple.WebCore                 0x0000000109146ada WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) + 618 (RenderBlockFlow.cpp:557)
> 5   com.apple.WebCore                 0x00000001091456b7 WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) + 1191 (RenderBlockFlow.cpp:407)
> 6   com.apple.WebCore                 0x000000010887c55d WebCore::RenderBlock::layout() + 125 (RenderBlock.cpp:1318)
> 7   com.apple.WebCore                 0x0000000109148ebb WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) + 1323 (RenderBlockFlow.cpp:640)
> 8   com.apple.WebCore                 0x0000000109146ada WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) + 618 (RenderBlockFlow.cpp:557)

[Noam Rosenthal]

*** Bug 203589 has been marked as a duplicate of this bug. ***

[Noam Rosenthal]

Created attachment 399826 [details]
Patch

[Noam Rosenthal]

This bug makes the wikipedia infobox create horizontal scroll:
https://en.wikipedia.org/wiki/Normal_distribution

The patch is verified to fix that.

[Noam Rosenthal]

*** Bug 109462 has been marked as a duplicate of this bug. ***

[Noam Rosenthal]

(In reply to Alexey Proskuryakov from comment #6)
> This change made two regression tests assert:
> 
> fast/table/multiple-captions-crash.xhtml
> fast/table/multiple-captions-crash2.xhtml
> 
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0   com.apple.JavaScriptCore      	0x0000000105bf7daa WTFCrash + 42
> (Assertions.cpp:333)
> 1   com.apple.WebCore             	0x0000000108ae4712
> WebCore::RenderTable::updateLogicalWidth() + 3618 (RenderTable.cpp:334)
> 2   com.apple.WebCore             	0x0000000108ae53c9
> WebCore::RenderTable::layout() + 569 (RenderTable.cpp:437)
> 3   com.apple.WebCore             	0x0000000109148ebb
> WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&,
> WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&,
> WebCore::LayoutUnit&) + 1323 (RenderBlockFlow.cpp:640)
> 4   com.apple.WebCore             	0x0000000109146ada
> WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) +
> 618 (RenderBlockFlow.cpp:557)
> 5   com.apple.WebCore             	0x00000001091456b7
> WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) + 1191
> (RenderBlockFlow.cpp:407)
> 6   com.apple.WebCore             	0x000000010887c55d
> WebCore::RenderBlock::layout() + 125 (RenderBlock.cpp:1318)
> 7   com.apple.WebCore             	0x0000000109148ebb
> WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&,
> WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&,
> WebCore::LayoutUnit&) + 1323 (RenderBlockFlow.cpp:640)
> 8   com.apple.WebCore             	0x0000000109146ada
> WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) +
> 618 (RenderBlockFlow.cpp:557)

There was a bug in the original patch from 2014 - (was modifying minPreferredWidth instead of maxPreferredWidth). The new patch should fix this.

[EWS]

Committed r261924: <https://trac.webkit.org/changeset/261924>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399826 [details].

[Radar WebKit Bug Importer]

<rdar://problem/63449424>