Issue metadata
Sign in to add a comment
|
Flexbox stopped working with with calc(). Breaking Google I/O web app |
||||||||||||||||||||||
Issue descriptionVersion: 52.0.2723.0 OS: Mac OS X There's a regression (or change) in flexbox that causes breakage on the Google I/O 2016 web app. Visit https://events.google.com/io2016/schedule?sid=a0d50c09-0cef-e511-a517-00155d5066d7#day1/a0d50c09-0cef-e511-a517-00155d5066d7 The session speakers are flex children, each having width: calc(50% - 32px). In chrome 50, the two speakers are correctly side-by-side. In Chrome 52 (and 51?), the speakers take up the full width of the overlay. Parent has: display: flex; flex-wrap: wrap; flex-direction: row; justify-content: space-between; Screenshots attached.
,
May 3 2016
Yea, sorry should have pointed that out. Safari and FF both display correctly (same as Chrome 50). Did not try Edge.
,
May 3 2016
You are probably looking for a change made after 386044 (known good), but no later than 386048 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7a247ee1edbb291fbbb5377a3b2073e1e83ce803..022699588a47674a7255f23daaa0574c8e91061e Yes, also rego's change. Still investigating whether this should work or not
,
May 4 2016
If it's related to my change I think it's normal that the behavior in Firefox is different. Again I'm only sure about Grid Layout, but Firefox behavior differs from Chrome there, I didn't get feedback from the CSS WG yet about which is the expected result though: https://lists.w3.org/Archives/Public/www-style/2016Apr/0329.html Dunno if this very same thing affects Flexbox or not, for example I'm wondering what's the expected behavior of something like this: <div style="display: flex; position: absolute;"> <div style="width: 50%; background: lime;">item</div> </div> In Chrome, it takes the whole with of the word "item", however in Firefox it uses the 50% like in a regular block. You can check it live in this jsbin: http://jsbin.com/fenuzo/1/edit?html,css,output I'll ping CSS WG to verify the expected behavior for flexboxes too.
,
May 4 2016
,
May 4 2016
In http://jsbin.com/fenuzo/1/edit?html,css,output, my expectation is that giving element width: 50% will make 50% of the parent :) However, that's not what 52 does, so that's unexpected. IMO, if the working group doesn't have answers, we should align with what the other browsers are doing. Chrome is the odd ball out now.
,
May 4 2016
Oh... that's surprising, I had other testcases where this didn't work for regular blocks either. I wonder how they differed, hmm
,
May 4 2016
The reason, btw, why this is problematic is because the position: absolute here shrinkwraps. But how do you calculate a shrinkwrapped size with a percentage? I guess what you're asking for is to ignore the percentage to size the parent, but apply the percentage when sizing the child?
,
May 4 2016
It worked great before :)
,
May 4 2016
My other testcase accidentally used inlines instead of blocks, which is why their percentage was ignored :( OK, I agree that we should match blocks. But this is still curious, I don't think that behavior (our old flex behavior / our still-current block behavior) actually matches what the spec requires.
,
May 4 2016
SGTM. FTR, I think we should be 100% spec compliant. My only plea is that we work coordinate with the other vendors for these kinds of updates. Flexbox has been one of those thorny toads over the years, and it continues as we work out these edge cases. In 2016, it's still very hard for a dev to write code that works across browsers :\. Luckily the big changes have settled, but smaller churn can still be detrimental to a layout.
,
May 4 2016
Rego - I would suggest reverting your change for now so that flexbox (and grid) can match regular blocks, what do you think? Talking to fantasai it sounded like it will be a little while until they resolve it.
,
May 5 2016
@cbiesinger I'm fine with reverting this for now as it seems it's not clear what should be the expected behavior. Anyway, I believe we've to revert 2 patches: * Positioned: https://codereview.chromium.org/1383003002/ * Floated: https://codereview.chromium.org/1892813002/
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec80ed8bf97986c752629bd0a550800fbe8bd82b commit ec80ed8bf97986c752629bd0a550800fbe8bd82b Author: rego <rego@igalia.com> Date: Thu May 05 09:35:36 2016 Revert of [css-grid] Floated grid containers have indefinite width (patchset #1 id:1 of https://codereview.chromium.org/1892813002/ ) Reason for revert: This is breaking how percentages work in Flexbox and Grid compared to regular Blocks. There're some discussion ongoing on the CSS WG to verify the expected behavior, so we're reverting this for now until we've a final resolution. Original issue's description: > [css-grid] Floated grid containers have indefinite width > > Fix LayoutBox::hasDefiniteLogicalWidth() to return FALSE for > floated grid containers with auto width. > > This makes that percentage tracks are treated as "auto" > for floated grid containers with indefinite width. > > BUG= 603854 > TEST=fast/css-grid-layout/floated-grid-container-percentage-tracks.html > > Committed: https://crrev.com/a7f443f6a36a2a524c4c29a2ef1bb6d02249d7b2 > Cr-Commit-Position: refs/heads/master@{#388054} TBR=cbiesinger@chromium.org,mstensho@opera.com,svillar@igalia.com # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 538513 , 603854 , 604346 , 608783 Review-Url: https://codereview.chromium.org/1948203004 Cr-Commit-Position: refs/heads/master@{#391782} [delete] https://crrev.com/5f1b82e8460a3d4d0a45346cdcdf5862be27c2c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/floated-grid-container-percentage-tracks.html [modify] https://crrev.com/ec80ed8bf97986c752629bd0a550800fbe8bd82b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83e80002d146d1493ae16c1cb585cc458e955ee3 commit 83e80002d146d1493ae16c1cb585cc458e955ee3 Author: rego <rego@igalia.com> Date: Thu May 05 12:18:13 2016 Revert of [css-grid] Fix definite/indefinite size detection for abspos elements (patchset #2 id:20001 of https://codereview.chromium.org/1383003002/ ) Reason for revert: This is breaking how percentages work in Flexbox and Grid compared to regular Blocks. There're some discussion ongoing on the CSS WG to verify the expected behavior, so we're reverting this for now until we've a final resolution. Original issue's description: > [css-grid] Fix definite/indefinite size detection for abspos elements > > We were considering that any abspos element has a definite size, and > that's not true. That's only true if the offset properties in that > dimension (left and right or top and bottom) are non-auto. > Fixed LayoutBox::hasDefiniteLogicalWidth|Height() to check this > properly. > > This has been confirmed by the CSS WG in the following thread: > https://lists.w3.org/Archives/Public/www-style/2015Nov/0074.html > > BUG= 538513 > TEST=fast/css-grid-layout/positioned-grid-container-percentage-tracks.html > > Committed: https://crrev.com/b75b9bdaa3ede6fe0230ae822885d6fb38ed514b > Cr-Commit-Position: refs/heads/master@{#386045} TBR=cbiesinger@chromium.org,svillar@igalia.com,mstensho@opera.com # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 538513 , 603854 , 604346 , 608783 Review-Url: https://codereview.chromium.org/1954683002 Cr-Commit-Position: refs/heads/master@{#391789} [delete] https://crrev.com/16d0abdd7230c89c7d0bf55e8de24a86fed39f40/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-percentage-tracks.html [modify] https://crrev.com/83e80002d146d1493ae16c1cb585cc458e955ee3/third_party/WebKit/Source/core/layout/LayoutBox.cpp
,
May 5 2016
Hopefully this has been fixed after the reverts.
,
May 5 2016
,
May 6 2016
Your change meets the bar and is auto-approved for M51 (branch: 2704)
,
May 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4cac97891d80a3f9789d4c5ae02a40ca4c9d3c21 commit 4cac97891d80a3f9789d4c5ae02a40ca4c9d3c21 Author: Christian Biesinger <cbiesinger@chromium.org> Date: Fri May 06 17:24:14 2016 Revert of [css-grid] Fix definite/indefinite size detection for abspos elements (patchset #2 id:20001 of https://codereview.chromium.org/1383003002/ ) Reason for revert: This is breaking how percentages work in Flexbox and Grid compared to regular Blocks. There're some discussion ongoing on the CSS WG to verify the expected behavior, so we're reverting this for now until we've a final resolution. Original issue's description: > [css-grid] Fix definite/indefinite size detection for abspos elements > > We were considering that any abspos element has a definite size, and > that's not true. That's only true if the offset properties in that > dimension (left and right or top and bottom) are non-auto. > Fixed LayoutBox::hasDefiniteLogicalWidth|Height() to check this > properly. > > This has been confirmed by the CSS WG in the following thread: > https://lists.w3.org/Archives/Public/www-style/2015Nov/0074.html > > BUG= 538513 > TEST=fast/css-grid-layout/positioned-grid-container-percentage-tracks.html > > Committed: https://crrev.com/b75b9bdaa3ede6fe0230ae822885d6fb38ed514b > Cr-Commit-Position: refs/heads/master@{#386045} TBR=cbiesinger@chromium.org,svillar@igalia.com,mstensho@opera.com BUG= 538513 , 603854 , 604346 , 608783 Review-Url: https://codereview.chromium.org/1954683002 Cr-Commit-Position: refs/heads/master@{#391789} (cherry picked from commit 83e80002d146d1493ae16c1cb585cc458e955ee3) Review URL: https://codereview.chromium.org/1957693002 . Cr-Commit-Position: refs/branch-heads/2704@{#423} Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251} [delete] https://crrev.com/179b1ad323b6bd741391eced5868c7876d65f536/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-percentage-tracks.html [modify] https://crrev.com/4cac97891d80a3f9789d4c5ae02a40ca4c9d3c21/third_party/WebKit/Source/core/layout/LayoutBox.cpp
,
May 6 2016
Testcase works for me now.
,
May 6 2016
Thanks for the fast turnaround on this!
,
May 11 2016
Issue 610714 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by cbiesin...@chromium.org
, May 3 2016