New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 608783 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Flexbox stopped working with with calc(). Breaking Google I/O web app

Project Member Reported by ericbidelman@chromium.org, May 3 2016

Issue description

Version: 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.
 
stable.png
848 KB View Download
canary.png
766 KB View Download
Labels: Needs-Bisect
Might be a duplicate of  bug 604346 , as these are inside a position: fixed element with indefinite size

But I haven't fully gone through the page structure yet; this may also be working correctly as it is now as there may be nothing to resolve the percentage relative to.

Does Firefox and/or Edge display this the way you want?
Yea, sorry should have pointed that out. Safari and FF both display correctly (same as Chrome 50). Did not try Edge.
Cc: r...@igalia.com
Labels: -Needs-Bisect
Owner: cbiesin...@chromium.org
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

Comment 4 by r...@igalia.com, 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.

Comment 5 by e...@chromium.org, May 4 2016

Status: Assigned (was: Untriaged)
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.
Oh... that's surprising, I had other testcases where this didn't work for regular blocks either. I wonder how they differed, hmm
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?
It worked great before :)
Cc: tabatkins@chromium.org
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.
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.
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.

Comment 13 by r...@igalia.com, 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/

Project Member

Comment 14 by bugdroid1@chromium.org, 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

Project Member

Comment 15 by bugdroid1@chromium.org, 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

Comment 16 by r...@igalia.com, May 5 2016

Hopefully this has been fixed after the reverts.
Labels: Merge-Request-51

Comment 18 by tin...@google.com, May 6 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 19 by bugdroid1@chromium.org, May 6 2016

Labels: -merge-approved-51 merge-merged-2704
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

Status: Fixed (was: Assigned)
Testcase works for me now.
Thanks for the fast turnaround on this!
Cc: msten...@opera.com cbiesin...@chromium.org nyerramilli@chromium.org
 Issue 610714  has been merged into this issue.

Sign in to add a comment