CSS tables within a flex container are rendering differently |
|||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.89 Safari/537.36 Example URL: https://jsbin.com/bipozoyuhu/edit?html,css,output Steps to reproduce the problem: I've built a test case here; it assumes the rendered area is too small to fit the strings being displayed without scrolling/wrapping: https://jsbin.com/bipozoyuhu/edit?html,css,output What is the expected behavior? With Chrome version 61.0.3163.100, the flex container size is increased to fit the strings without wrapping, and it becomes scrollable. What went wrong? With my current version of Chrome, the flex container displays the tables such that they overlap. Does it occur on multiple sites: N/A Is it a problem with a plugin? No Did this work before? Yes 61.0.3163.100 Does this work in other browsers? N/A Chrome version: 62.0.3202.89 Channel: stable OS Version: OS X 10.12.6 Flash Version: FWIW, I'm not convinced the new behavior is incorrect, but it's definitely different. At the time of writing, the latest version of Safari matches Chrome 61.0.3163.100 (content scrolls), whereas the latest version of Firefox wraps the text (rather than overlapping).
,
Nov 9 2017
Able to reproduce the issue on the reported chrome version stable 62.0.3202.89 and on the latest dev 64.0.3262.0 using Windows10, Ubuntu 14.04 and Mac 10.12.6. Manual Bisect Info: --------------------------- Good Build: 62.0.3169.0 Bad Build: 62.0.3170.0 You are probably looking for a change made after 490404 (known good), but no later than 490405 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/d1eb577c332b15ba22caf53ecbbd06093df5d633..05f74c2faaa2daac62a38d3d03b56835eff212c9 Reviewed-on: https://chromium-review.googlesource.com/590729 Suspecting same from changelog. @cbiesinger:Please confirm the issue and help in re-assigning if it is not related to your change.
,
Nov 9 2017
Recent regression, can we get fix for M64? Please feel free to modify the RB label accordingly.
,
Nov 10 2017
,
Nov 20 2017
At first sight it looks like Firefox is correct, but I'm still investigating
,
Nov 20 2017
,
Nov 20 2017
Oh this is a fun bug. dgrogan, please see question at the bottom. Sooo, this is what happens: We compute a desired width for the flex items (ie. the tables) as half the container width each, like you'd intuitively expect (flex-basis is 0 for each and the flex-grow factor is the same). We set this as the override main axis size (here, the override logical width) BUT, it turns out that LayoutTable::UpdateLogicalWidth overrides the base class version of this function and ignores the override width (!) Now, this kinda-sorta used to work (not really, because the size was still wrong). This is the important change: - LayoutUnit child_main_extent = MainAxisExtentForChild(*child); + LayoutUnit child_main_extent = flex_item.FlexedBorderBoxSize(); That is -- we used to check the actual child.Width() for positioning the children, whereas now we just use the size we computed that the child *should* have. Of course, they ought to be the same. dgrogan, is there a reason why LayoutTable::UpdateLogicalWidth does not take the override width into account / should it be changed to do so?
,
Nov 21 2017
Issue 784680 has been merged into this issue.
,
Nov 21 2017
In the meantime I made https://chromium-review.googlesource.com/#/c/chromium/src/+/780462 to fix this regression/release blocker
,
Nov 21 2017
I do not know off the top of my head. Maybe Morten does? Looks like the opposite problem was noticed but not solved yet due to compat issues in issue 667785. We can talk more after the US holiday break? /me updates OOO status
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7a360904e8d506c17894c47cf482cb42335e426 commit c7a360904e8d506c17894c47cf482cb42335e426 Author: Christian Biesinger <cbiesinger@chromium.org> Date: Tue Nov 21 05:26:39 2017 [css-flexbox] Re-read a flex item size after layout ...because tables ignore the override width, and if we don't re-check the size we position the flex items incorrectly. R=mstensho@chromium.org Bug: 782948 Change-Id: I86ecf63fe2d9cd6b27813436a842e15cfed783c6 Reviewed-on: https://chromium-review.googlesource.com/780462 Reviewed-by: David Grogan <dgrogan@chromium.org> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#518135} [add] https://crrev.com/c7a360904e8d506c17894c47cf482cb42335e426/third_party/WebKit/LayoutTests/css3/flexbox/bug782948.html [modify] https://crrev.com/c7a360904e8d506c17894c47cf482cb42335e426/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
,
Nov 21 2017
When a table is a flex item (or grid item?), it needs to give up at least some of its own sizing peculiarities, and let the flex (/grid?) algorithm decide more. Would it work if LayoutTable::UpdateLogicalWidth() just calls super::UpdateLogicalWidth() and returns, if it's a flex (/grid?) item?
,
Nov 21 2017
Hmm interesting. That's probably a good idea, however I would suggest that we wait with that until after the upcoming branch so there's more time to catch any potential site breakage from that? Meanwhile I'd like to take my patch for merging to the branch as that restores our previous behavior (and this is a release blocker)
,
Nov 21 2017
This bug requires manual review: We are only 13 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 21 2017
This was regressed in M62 (not M63 regression). M63 is very close to Stable promotion and we're only taking critical merges in. Hence, I'm rejecting merge to M63. Please let me know if there is any concern here.
,
Nov 22 2017
,
Dec 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eee88582b5300e34aa2ea56bc5da17457e67aa00 commit eee88582b5300e34aa2ea56bc5da17457e67aa00 Author: Morten Stenshorne <mstensho@chromium.org> Date: Thu Dec 07 12:45:27 2017 Avoid table sizing peculiarities for flex and grid items. Let the flex/grid layout algorithm determine the size of tables that are flex/grid items. Don't let the intrinsic width of table columns affect the width of the item. Bug: 782948 , 667785 Change-Id: Ifcea6af51ce5bf74377f93b5cd0437272a5c7ef0 Reviewed-on: https://chromium-review.googlesource.com/781859 Reviewed-by: David Grogan <dgrogan@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#522408} [delete] https://crrev.com/8448affe3471114a8be3a25198083c6f8ab3f103/third_party/WebKit/LayoutTests/css3/flexbox/bug782948.html [delete] https://crrev.com/8448affe3471114a8be3a25198083c6f8ab3f103/third_party/WebKit/LayoutTests/css3/flexbox/flexitem-expected.txt [modify] https://crrev.com/eee88582b5300e34aa2ea56bc5da17457e67aa00/third_party/WebKit/LayoutTests/css3/flexbox/flexitem.html [add] https://crrev.com/eee88582b5300e34aa2ea56bc5da17457e67aa00/third_party/WebKit/LayoutTests/external/wpt/css/css-flexbox/table-as-item-narrow-content.html [add] https://crrev.com/eee88582b5300e34aa2ea56bc5da17457e67aa00/third_party/WebKit/LayoutTests/external/wpt/css/css-flexbox/table-as-item-wide-content.html [add] https://crrev.com/eee88582b5300e34aa2ea56bc5da17457e67aa00/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/explicitly-sized-grid-item-as-table.html [add] https://crrev.com/eee88582b5300e34aa2ea56bc5da17457e67aa00/third_party/WebKit/LayoutTests/external/wpt/css/reference/ref-filled-green-100px-square-only.html [modify] https://crrev.com/eee88582b5300e34aa2ea56bc5da17457e67aa00/third_party/WebKit/Source/core/layout/LayoutTable.cpp |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by manoranj...@chromium.org
, Nov 8 2017Labels: Needs-Bisect Needs-Triage-M62