New issue
Advanced search Search tips
link

Issue 927066: Flexbox rendering changed between chrome 71 and 72

Reported by eld...@gmail.com, Jan 30

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce the problem:
Minimal example of different rendering behavior I ran into: https://jsfiddle.net/z5wn6byv/

What is the expected behavior?

What went wrong?
As you can see from the attached images, the rendering changed between Chrome 71 and 72. In my case, it was around using some nested flex containers, each of which had height: 100%. Somewhat silly use case but not something you would expect to change between versions.

Did this work before? Yes 

Does this work in other browsers? N/A

Chrome version: 72.0.3626.81  Channel: n/a
OS Version: 10.0
Flash Version: 

A couple others reported flexbox issues as discussed here, though not sure if they were from the same exact issue I ran into:
 https://www.reddit.com/r/webdev/comments/alg8az/anyone_experiencing_issues_with_flexbox_and/
 
Chrome 71 flexbox.PNG
1.9 KB View Download
Chrome 72 flexbox.png
2.9 KB View Download
Showing comments 17 - 116 of 116 Older

Comment 17 by kel...@gmail.com, Feb 1

This change broke quite a bit on the a few of the commercial web apps I work on.  We knew about it ahead of time, since a few of us run the beta chrome, but still a regression in my opinion.  Ours might not qualify as a "real" website, whatever that is, but sure has been a pain.

Comment 18 by anononon...@gmail.com, Feb 1

"I don't think we got a report of this on a real website" -
continuing jack's comment on his app and the comments above me - my company's app got broken as well.
ever since this issue was reported, all front-end teams in my company are working on testing and patching the workaround for every flex usage we have.

Given the facts that:
1. this commit changes flexbox behavior from 72.0.3589.0 and on (regression).
2. chrome browser update itself automatically.
3. real websites/apps are being affected by this issue.
I think we should handle this case a bit different.

For my point of view, we should roll out a fix to stop current and future breaks of any websites\apps.

In the meanwhile, a different discussion should be held regarding the interop with Firefox.
And if, eventually, the interop is more important than keeping on the same behavior of flexbox, I would expect chromium to post a release note for this case (https://chromereleases.googleblog.com/2019/01/stable-channel-update-for-desktop.html)

Comment 19 by sobreiro...@gmail.com, Feb 1

Hi!

Found the same problem on our MES application. (https://criticalmanufacturing.com)

Although it's good to approximate Chromium with Firefox and other browsers, the fact is that the example shown on https://jsfiddle.net/z5wn6byv/ works well with Firefox, Edge and Safaria, and not with Chromium. So, in fact we diverged here.

Any chance this could be fixed soon?

Comment 20 by e...@chromium.org, Feb 1

Hi everyone, thanks for bringing this to our attention.

When Christian said that we haven't seen this affect real websites in comment ten that was by no mean an indication that some sites aren't real or less important. He was merely stating that it was a theoretical problem and that we had not yet seen any sites affected by it.
We're now well aware that this is breaking a number of sites and we're considering ways to mitigate it.

This is marked as a high priority issue and we're actively working on it. Thanks for your patience and reports!

Comment 21 by aodhan.h...@gmail.com, Feb 1

We've also seen a regression across our entire application. We saw this coming but just didn't have the man power to complete all of the styling changes before 72 rolled. Because Firefox was also behaving the same way I was skeptical it was a bug, I thought Chrome may have refined there implementation to be more to spec, or something. Any mitigation appreciated, thanks for all the work you guys do.

Comment 22 by j...@logichub.com, Feb 1

At the very least, please include a potential breaking change like this in the release notes.

Comment 23 by dneelame...@chromium.org, Feb 1

we are seeing this issue on Samsang S6 edge/NRD90M/ on webview : 72.0.3626.86

Comment 24 by jcollins@google.com, Feb 1

This issue indirectly broke webkit scrolling on all Dartdoc generated websites (api.dartlang.org, docs.flutter.io).   An example page:  

https://docs.flutter.io/flutter/rendering/rendering-library.html

In 71 this scrolled independently in all three columns, but scrolls as a single page in 72.

The workaround above, applying min-height: 0, does work but it will be some time before I can get that pushed throughout the ecosystem.  Dartdoc's issue for this is https://github.com/dart-lang/dartdoc/issues/1921.

Comment 25 by abdulsyed@google.com, Feb 1

Issue 927807 has been merged into this issue.

Comment 26 by josh_tay...@esri.com, Feb 1

I am seeing this issue as well on an app that I am working on after updating to chrome 72.

Comment 27 by trumb...@google.com, Feb 1

Labels: Hotlist-ConOps-CrOS

Comment 28 Deleted

Comment 29 by tom.wilt...@gmail.com, Feb 1

Hi Enne!

For whatever it's worth, this affected our (Welkin's) web app, too.

Our fix, as others have reported, was using min-height: 0 on the container. Worked well.

Thanks for all your hard work as always!

- Tom

Comment 30 by cbiesin...@chromium.org, Feb 2

So I think there are a few things going on in this bug and they are difficult to untangle.

Firefox's behavior here is according to the spec; the intention was to match Firefox. People who saw that their site is now broken, could you specify whether it works in Firefox or not?

Independent of that, there is a different issue with percentage heights. If you see this issue, especially if Chrome behaves different from Firefox, could you specify whether you are using percentage heights?

Finally, you are correct that this should have been mentioned in the blog announcement, I will see about correcting that.

Comment 31 by anononon...@gmail.com, Feb 2

Hi there,

Regarding the broken sites, whether it works in Firefox or not?
Yes indeed. Chrome behaves different from Firefox.
Comparing Chrome 72.0.3626.81 to Firefox 65.0 (both are the latest versions as for now)

Whether you are using percentage heights?
Yes we do. Nested element has "height: 100%".

The testcase from comment 0 might help as it answers both of your questions:
https://jsfiddle.net/z5wn6byv/

Comment 32 by anononon...@gmail.com, Feb 2

BTW, Considering the areas this issue affects and comment #20 saying it's a high priority issue,
May I suggest to change the issue's Pri from 1 (Need for target milestone) to 0?

Comment 33 by ahuva.ha...@sisense.com, Feb 4

This affected our site, too!
We've been busy working on fix packs for the past few days.  As we have both a managed cloud service and on-prem deployments, this is causing a lot of people a lot of work...

Comment 34 by aroshaja...@gmail.com, Feb 4

This has also broken our application. 

We also use flex layouts across our application. This has caused chaos with the layouts that we use. It has broken nested layouts. 

We use flex extensively in complex visual setups such as gantt's and dashboard widgets. 

There are image issues as well. We use a lot of SVG's. Essentially this new update caused all percentage css sizing applied to SVG's to be ignored. We now have all of our developers going through all images used across our application and applying absolute sizing which is a disaster for maintainability. 

This has cost our company a lot of time and has damaging impact on our reputation with our clients. Chrome automatically updates so we can't even ask clients not to upgrade until issues have been resolved.
This problem has come up several times when a commit is added to try "align with firefox". It isn't the first time flex has been changed in a breaking way for this reason.

So, can someone who understands the aim here thoroughly please explain _why_?

* Why does `min-height: 0` solve the problem?
* Which element needs this to solve the problem? (I assumed the flex box, but sometimes it seems to be the parent, and other times it seems to be the parent's parent. no consistency)
* Why was this change introduced at all? You're trying to align to firefox, so why did they introduce it? An explanation as to what the root of this change was would be nice and why that was made

Currently, people are just throwing min-heights all over the place without any clue why, because it fixes the problem. I think an explanation to all of this would go a long way and help people understand why this strange hack works.

Comment 36 by sscheinh...@luxfts.com, Feb 4

This is affecting our application as well. I can confirm our flex layout is working in Firefox and broken in Chrome as of v72. 

So this is definitely a bug.

Comment 37 by brandlu...@googlemail.com, Feb 4

For what it’s worth, this is a breaking change for our corporate rich-web application, which was productively using flex layouting for over a year. 
I noticed that throwing in a flex-basis: 100%; element makes things considerably worth, as that still seems to fill the parent container just fine, whereas the succeeding flex-basis: auto element gets reduced to a few px height. min-height:0 seems to fix it, however that change results in inoperable scroll bars a few elements deeper inside the nesting, unless every element inbetween gets overflow hidden (or display: contents, but let’s not talk about that)

Due to the nature of the web app, creating a minimal example codepen is next to useless.

Comment 38 Deleted

Comment 39 by cbiesin...@chromium.org, Feb 4

OK, it sounds like percentages are very common, so I will create a fix for that case.

(The min-height: 0 should be added to the flex item)

The reason this was introduced is as follows: Flex items, in general, have an automatic min-size: min-content. The details are bit more complicated, but Chrome *did* implement this for most cases. The only case for which Chrome did not implement this was when the flexbox is a column flexbox and the child is also a flexbox. This was to avoid triggering a (different) bug. Clearly, treating child flexboxes different from regular child boxes is weird, so I fixed this bug. Unfortunately, as this bug report shows, there was still the issue of percentage sizing.

I will work on a temporary fix to not apply this min-height: min-content to flex items which have percentage-sized children. I apologize for causing this issue.
Thanks for the brief explanation.

We don't use percentages but do have a `100vh` on the very top level element (below body), would the same issue apply there?

To fix it in our case, I had to apply `min-height: 0` to most of the elements in the tree (a tree roughly 5-6 elements deep where each parent is a flex box). This still feels rather hacky... can you explain why setting the value to `0` works? Maybe that would make it more acceptable of a solution.

The height isn't going below zero, so why does setting the minimum to zero sort it? Explaining this would do wonders so it seems less like a hack.

Comment 42 by anononon...@gmail.com, Feb 4

Thanks for the your response Christian.

As for now, this item is marked on Priority 1 = Need (for target milestone).
Considering the comments above and the high usage of percentages - is there any change this fix will be marked on higher priority?

It will be highly appreciated if the fix rolls out before the next target milestone.

Comment 43 by cbiesin...@chromium.org, Feb 4

Labels: -Pri-1 Pri-0
I mean, this is marked as release-block-stable, and I am actively working on this, but sure, I can also mark it P0

Comment 44 by djmm@google.com, Feb 4

How critical is this for OS?

Comment 45 by abdulsyed@google.com, Feb 4

djmm@ - Critical for OS as well.

Comment 46 by jcgilmo...@gmail.com, Feb 4

Most of the issues we saw were fixed by adding overflow: hidden to the flex parent itself. But, I've only started scratching the surface. I've compartmentalized all of the fixes in a SCSS partial to easily pull out if (hopefully) a patch is released to address the issue.

Comment 47 by abdulsyed@google.com, Feb 4

Cc: cbiesin...@chromium.org susan.boorgula@chromium.org
 Issue 928002  has been merged into this issue.

Comment 48 by bugdroid, Feb 4

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f73107eed2d8f61e189dcd52239159129961750

commit 4f73107eed2d8f61e189dcd52239159129961750
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Mon Feb 04 18:44:26 2019

[css-flexbox] Don't apply min-height: auto to children with percentage heights

In such a case, we compute an incorrect intrinsic logical height, because
we resolve pre-flex percentages when we calculate the intrinsic height.
In common cases this can be 100% of the flexbox height, which we later
use as a min-height of the flex item, leading to it not shrinking when
it should.

This reverts https://crrev.com/c/1269235 for the case of a flex item
which is a flexbox with percentage children.

Bug: 927066
Change-Id: I955bf651f75495122bb454d5221303b188bbb03c
Reviewed-on: https://chromium-review.googlesource.com/c/1452477
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628806}
[modify] https://crrev.com/4f73107eed2d8f61e189dcd52239159129961750/third_party/blink/renderer/core/layout/flexible_box_algorithm.cc
[add] https://crrev.com/4f73107eed2d8f61e189dcd52239159129961750/third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-minimum-height-flex-items-012.html

Comment 49 by bugdroid, Feb 4

Project Member
Labels: merge-merged-3693
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce9e9ed14a8a4e417af3bf967fee3a467131d137

commit ce9e9ed14a8a4e417af3bf967fee3a467131d137
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Mon Feb 04 18:47:33 2019

[css-flexbox] Don't apply min-height: auto to children with percentage heights

In such a case, we compute an incorrect intrinsic logical height, because
we resolve pre-flex percentages when we calculate the intrinsic height.
In common cases this can be 100% of the flexbox height, which we later
use as a min-height of the flex item, leading to it not shrinking when
it should.

This reverts https://crrev.com/c/1269235 for the case of a flex item
which is a flexbox with percentage children.

Bug: 927066
Change-Id: I955bf651f75495122bb454d5221303b188bbb03c
Reviewed-on: https://chromium-review.googlesource.com/c/1452477
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#628806}(cherry picked from commit 4f73107eed2d8f61e189dcd52239159129961750)
Reviewed-on: https://chromium-review.googlesource.com/c/1452490
Reviewed-by: Abdul Syed <abdulsyed@google.com>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3693@{#6}
Cr-Branched-From: 58322e3f2f0c240b91b56bc9039cd989b201b557-refs/heads/master@{#628647}
[modify] https://crrev.com/ce9e9ed14a8a4e417af3bf967fee3a467131d137/third_party/blink/renderer/core/layout/flexible_box_algorithm.cc
[add] https://crrev.com/ce9e9ed14a8a4e417af3bf967fee3a467131d137/third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-minimum-height-flex-items-012.html

Comment 50 by cbiesin...@chromium.org, Feb 4

james.garbutt: Are you saying your website works in Firefox but not in Chrome 72, and you're using 100vh? The 100vh value is probably unrelated.

The reason why it fixes things is that with the default (min-height: auto), we would calculate the "intrinsic" height of the flex item and use that as the minimum, below which it would not shrink. This is the behavior in the spec, in Chrome 72, and in Firefox. For example, if you have 3 lines of text, we would compute a minimum height of 54px or so (depending on your font height, line spacing, etc.); for nested flex items we add that up recursively. For certain applications such a minimum size makes no sense, so min-height: 0 is the correct fix. (The reason this exists at all is so that you don't accidentally shrink your flex item size below its content size and have it overflow, particular because flex-shrink: 1 is the default value)

jcgilmore2: Hm, that may work too, but min-height: 0 on flex items should get you exactly the old behavior

Comment 51 by gov...@chromium.org, Feb 4

Thank you cbiesinger@ for fix and Thank you abdulsyed@ for triggering new canary with the merge.

+candrada@, requesting test team to verify this bug on canary version 74.0.3693.3+.

Comment 52 by candr...@chromium.org, Feb 4

Cc: aska...@chromium.org
+askatte to verify on Android

Comment 53 by zak.stra...@visor.com, Feb 4

When can a new version of chrome be expected? I'm trying to figure out whether I should manually make fixes across the site but not worth the effort if it will be fixed tomorrow.

Comment 54 by abdulsyed@google.com, Feb 5

Hi All - can you please help us by testing your sites/apps on the latest version of Canary?

https://www.google.com/chrome/canary/

The fix has landed there.

Comment 55 by abdulsyed@google.com, Feb 5

This will be on canary version: 74.0.3693.4

Comment 56 Deleted

Comment 57 by pucchakayala@chromium.org, Feb 5

The fix is working correctly on Win 10 / 64 bit - 74.0.3693.4 Canary build
Please find the attached screen shot.
927066 on M74 Canary Windows.PNG
3.0 KB View Download

Comment 58 by jacklu...@gmail.com, Feb 5

Just tested on 

Canary
Version 74.0.3693.4 (Official Build) canary (64-bit)

The fix is not working for me. Removing `min-height: 0px;` from our css causes scrollbars to disappear.

I am using Opera to test older Chrome and can confirm that flex is behaving correctly there.

I've attached an image of our UI just to give an idea of the layout containers it is rendering with broken and working scrollbars.

Comment 59 by jacklu...@gmail.com, Feb 5

Sorry proper screenshots here
file_detail(scrollbars working).png
711 KB View Download
file_detail(broken scrollbars).png
697 KB View Download

Comment 60 by pucchakayala@google.com, Feb 5

Working fine on Mac OSX 10.13.6 Canary Build # 74.0.3693.3
927066 on Mac OSX M74 Canary.png
12.8 KB View Download

Comment 61 by gov...@chromium.org, Feb 5

Labels: M-73

Comment 62 Deleted

Comment 63 by jcgilmo...@gmail.com, Feb 5

Ok, so our issue was a little unique. We rely a lot on flex columns to create elements likes drawers, panels, cards, etc that contain dynamic body areas that allow for elements above and below (like headers and footers above and below a list). This creates a really dynamic and responsive element that will grow/shrink based on available space. We create this "dynamicBody" by adding flex-grow: 1 along with overflow: auto. However - sometimes our devs introduce extra elements that break the flex parent/item relationship we rely on and a common hack has been to add a "column" class in between so that those nested items remain flex items. 

Apparently the issue that this Chrome "bug" exposed is the same known bug in Firefox that we hadn't pulled in sprint yet. This Codepen shows our issue, as well as a few other examples (one without the extra column, one with height added to the extra column). I wrote some really quick crappy jquery to toggle the min-height and overflow so you can see it fix the scroll. So, although this bug did break a few areas, it exposed areas that we already needed to fix and now seems to be aligned with Firefox. Safari seems to be the lone wolf now, since the left most box still appears normal and scrolls in the example.

https://codepen.io/jcgilmore2/pen/PVKzQR

Thanks everyone for all the helpful replies, and the Chromium team for your work and responsiveness.
Fixed on our end too. As mentioned above, we don't use percentages but somehow the canary bugfix sorted it anyway.

Good fast response.

Comment 65 by phi...@chatlayer.ai, Feb 5

Any update on this?  Everything is broken

Comment 66 by kgna...@chromium.org, Feb 5

Android: works as per expected behavior, Issue is fixed on 74.0.3693.5

Comment 67 by ke...@faktion.com, Feb 5

Will there be a hotfix for people using version 72

Comment 68 by olivier....@gmail.com, Feb 5

Same issue here, and it break our application in production. Any chance Google can release an update fast ? Thanks !

Comment 69 by eduardo...@gmail.com, Feb 5

The "min-height: 0;" worked in our app but the fix applied in Canary did not.
Canary 74.0.3694.0 - Sample.png
29.2 KB View Download
Chrome 72.0.3626.81 - Sample.png
25.4 KB View Download
Firefox 66.0b4 - Sample.png
31.3 KB View Download

Comment 70 by adamru@google.com, Feb 5

Same issue with our application. Canary 74.0.3693.5 does not fix the issue.

Comment 71 by cr-audit...@appspot.gserviceaccount.com, Feb 5

Project Member
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 032ef9666487db1d04b656a095b041de8c6d2b63 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required --

Comment 72 by cr-audit...@appspot.gserviceaccount.com, Feb 5

Project Member
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/032ef9666487db1d04b656a095b041de8c6d2b63

Commit: 032ef9666487db1d04b656a095b041de8c6d2b63
Author: cbiesinger@chromium.org
Commiter: cbiesinger@chromium.org
Date: 2019-02-05 21:05:58 +0000 UTC

Revert "[css-flexbox] Apply min-height: auto to nested flexboxes again"

This reverts commit 155c1f2bf45369bc7130c89b88c75c2d3c915410.

Bug: 927066
Change-Id: Icfa8248cf68dbffa581c4da7477f457a8ab8fd10
Reviewed-on: https://chromium-review.googlesource.com/c/1455112
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#833}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Comment 73 by bugdroid, Feb 5

Project Member
Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/032ef9666487db1d04b656a095b041de8c6d2b63

commit 032ef9666487db1d04b656a095b041de8c6d2b63
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Tue Feb 05 21:05:58 2019

Revert "[css-flexbox] Apply min-height: auto to nested flexboxes again"

This reverts commit 155c1f2bf45369bc7130c89b88c75c2d3c915410.

Bug: 927066
Change-Id: Icfa8248cf68dbffa581c4da7477f457a8ab8fd10
Reviewed-on: https://chromium-review.googlesource.com/c/1455112
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#833}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/032ef9666487db1d04b656a095b041de8c6d2b63/third_party/blink/renderer/core/layout/flexible_box_algorithm.cc

Comment 74 by offirmo....@gmail.com, Feb 5

My hobby browser game is hit by the bug and Chrome canary 74.0.3694.0 doesn't fix it. It indeed uses nested flexboxes with width: 100%, starting straight from the top.

It works perfectly on Firefox latest.

Reproduction:
1. visit https://www.online-adventur.es/apps/the-boring-rpg/
2. click on the bottom left "crown with laurel" icon
3. the achievement list should be scrollable and should not overlap the bottom-left menu. This is broken right now. (was working in Chrome 71)

Attached file: what we should have (for reference), it was working in Chrome 71

I'm willing to hold on mitigating the issue so that the dev team can reproduce the issue. Please tell me if that's helpful.
www.online-adventur.es_apps_the-boring-rpg_(HD with bars) (1).png
3.7 MB View Download

Comment 75 by offirmo....@gmail.com, Feb 5

Correction to my previous report: it doesn't work on Firefox latest either. Also should read "height" instead of "width".

I'm surprised that a nested 100% flexbox element can escape its flexbox container.

Comment 76 by mglu...@gmail.com, Feb 5

@bugdroid thanks so much for fix!  Can you divulge when and for which version of chrome this will take effect? I’m working on overnight fixes for broken layouts in 72 for a large codebase now (which are having some side effects!). So if this reversion is forthcoming we would take that into our account for our plan.  Thank you!

Comment 77 by e...@chromium.org, Feb 5

Quick update:

We've heard you loud and clear and have decided to revert the change in Chrome 72 to avoid breaking existing sites and to allow developers a bit more time to update their pages.

The change will instead ship with Chrome 73. As such it's still important to make the required changes to avoid breaking it in future versions of Chrome and to ensure compatibility with Firefox.

We highly recommend that you test your sites in Chrome beta to ensure that they'll work in the next version and comment here or file a new bug if it's still broken after applying the suggested changes.

Thank you.

Comment 78 by gov...@chromium.org, Feb 6

Labels: -CommitLog-Audit-Violation -Merge-Without-Approval
This merge was approved during internal meeting.

Comment 79 by ahuva.ha...@sisense.com, Feb 6

Thank you for listening to customer feedback! Really appreciate your responsiveness.

Will you be releasing another build of Chrome 72 including reverting the change? When do you expect to release it?

We're in the process of updating all of our managed service servers, and if this is unneeded at this time, would be great to know ASAP.

Thanks!
Labels: TE-Verified-72.0.3626.96 TE-Verified-M72
Able to reproduce this issue on Windows 10, Mac 10.14 and Ubuntu 17.10 on the reported version 72.0.3626.81 and the issue is fixed on the latest M-72 build 72.0.3626.96 as per comment #0.

1. Launched chrome 
2. Opened given jsfiddle > "https://jsfiddle.net/z5wn6byv/". 
As we have observed that the Flexbox rendering correctly.Attached is the screenshot for reference.

Hence adding TE verified labels as the fix is working as expected.

Thanks..!!
927066.PNG
39.1 KB View Download

Comment 81 by duisters...@gmail.com, Feb 6

The https://jsfiddle.net/z5wn6byv/ shows a broken result in latest Chrome (72), not in Chrome (71). It does not show a broken result in Firefox.

However, there was a specific setup which uses a container with flex 1 1 auto with items inside of there with height 100% that did not have the expected behavior in either Chrome (56) nor any version of Firefox.

I read in the comments that there has been a change to flex to be more aligned with Firefox. The suggested fix of min-height: 0 on the container works IF you also put the children at height 100%.

tl;dr
Set min-height: 0; on the container with flex: 1 1 auto. Set height: 100% on the children of that container.

Comment 82 by duisters...@gmail.com, Feb 6

To add to comment 81.

To also have it working on IE11. Use height:100% instead of min-height: 0. This also works for Firefox and Chrome (72).

Comment 83 by kgna...@chromium.org, Feb 6

Android: Works as per expected behavior, Issue is verified on 72.0.3626.96

Comment 84 by mate.bal...@gmail.com, Feb 6

Chrome version: 72.0.3626.81

"The change will instead ship with Chrome 73. As such it's still important to make the required changes to avoid breaking it in future versions of Chrome and to ensure compatibility with Firefox."

If I understand right in Chrome 73 the attached image will be the expected behavior? (and we have to add min-height: 0 to the proper places to avoid this?)

Or what is the proper "required changes"? 

Thank you!
Chrome 72 flexbox.png
2.9 KB View Download

Comment 85 by sscheinh...@luxfts.com, Feb 6

"The change will instead ship with Chrome 73. As such it's still important to make the required changes to avoid breaking it in future versions of Chrome and to ensure compatibility with Firefox."

As I reported earlier in this thread, our application renders correctly in Firefox and is broken in Chrome as of v72. So there is absolutely a bug in the "Firefox parity fix" you guys have implemented.
Android Webview : Issue verified on M72: 72.0.3626.96 in the device Pixel XL / OPM1.190105.002

Comment 87 by mglu...@gmail.com, Feb 6

How do releases work for revisions to stable releases like 72? I guess what im asking specifically is when will the changed version of 72 be in android play store?  Cheers!

Comment 88 by pucchakayala@google.com, Feb 6

Steps followed as per # 81

Reproduction:
1. visit https://www.online-adventur.es/apps/the-boring-rpg/
2. click on the bottom left "crown with laurel" icon

Able to scroll through the list correctly & do not see any overlaps with the menu on latest stable build # 72.0.3626.96 [Win 10/64 bit & Mac OSX 10.13.6]

Comment 89 by dneelame...@chromium.org, Feb 6

Verified/Fix on Webview : M72/72.0.3626.96 device:Pixel 3/PD1A.180720.018

Comment 90 by aska...@google.com, Feb 6

Verified fix for https://www.online-adventur.es/apps/the-boring-rpg/ as well on Android 72.0.3626.96. Thanks

Comment 91 by songsuk@chromium.org, Feb 6

Verified fix with M72_flexbox_cros.png & Comment#88 on M72.0.3626.97/CrOS 11316.123.0 - Coral_santa, Snappy
m72_flexbox_cros.png
5.3 KB View Download

Comment 92 by cbiesinger@google.com, Feb 6

Hi all,

to recap: When the Chrome 72 rollout continues, it will have the behavior of Chrome 71 restored for this case

For 73, we would like to match the Firefox behavior (which is basically what Chrome Canary has). If you have a website that is broken in Canary/72.0.3626.81 but works in Firefox, it would be great if you could file a new bug report here (crbug.com/wizard), with a URL showing the problem. It does not have to be a jsbin/codepen URL, although that would be helpful. I would like to fix any such issues.

However, for websites where Chrome 72 matches Firefox, you should expect that Chrome 73 will render them like Firefox (even though we are reverting the change for the final rollout of 72), and so for such cases I would definitely encourage y'all to add the min-height: 0 as needed.

Comment 93 by cbiesin...@chromium.org, Feb 6

Labels: Merge-Request-73
(Please mention the bug number here when you file a new bug so I can track it more easily)


Release managers, I would like to merge the patch from Comment 49 to 73 to fix some of the reported problems here.

Comment 94 by abdulsyed@google.com, Feb 6

Labels: -Merge-Request-73 Merge-Approved-73
Approving #49 to 73. branch:3683

Comment 95 by aroshaja...@gmail.com, Feb 7

Hi

Can someone please clearly state when this will be reverted in production. I.e. when can we expect the end users browser version to change so this change has been reverted? 

We have a number of clients experiencing major issues because of this and we would like to go back with a fix date. I need to know if this will revert before my team manage to apply their own fixes. 

Thanks

Comment 96 by 11h...@gmail.com, Feb 7

Hi,
Thanks for fixing this issue!  I see that the 72.0.3626.96 release was announced for Android but that it would be available "over the course of the next few weeks ."

Should we expect that the rollout will be in roughly the same order as the previous v72 release?  E.g., a user who got the previous v72 after six days would have to wait about six more days to get this latest fix?  Is there a chance that some users might remain broken for three weeks?

Is there any way to accelerate this rollout?  Thanks!

Comment 97 Deleted

Comment 98 by yapaarac...@gmail.com, Feb 7

"I would definitely encourage y'all to add the min-height: 0"

Is this, "min-height: 0" solution will work for all scenarios. Because, I found that it doesn't work for some cases

Comment 99 by eduardo...@gmail.com, Feb 7

Tested with Canary 74.0.3694.0 and the issue is still happening. Created issue https://bugs.chromium.org/p/chromium/issues/detail?id=929690. Please let me know if some information is missing or if the test case is enough.

Comment 100 by rsgavara@google.com, Feb 7

Labels: ext-reporter

Comment 101 by cr-audit...@appspot.gserviceaccount.com, Feb 7

Project Member
Labels: -Merge-Approved-73 Merge-Merged-73-3683
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/7fa7ef3bfd087d66ddfb7cbd8ad7297ef8b09b51

Commit: 7fa7ef3bfd087d66ddfb7cbd8ad7297ef8b09b51
Author: cbiesinger@chromium.org
Commiter: cbiesinger@chromium.org
Date: 2019-02-07 19:33:09 +0000 UTC

[css-flexbox] Don't apply min-height: auto to children with percentage heights

In such a case, we compute an incorrect intrinsic logical height, because
we resolve pre-flex percentages when we calculate the intrinsic height.
In common cases this can be 100% of the flexbox height, which we later
use as a min-height of the flex item, leading to it not shrinking when
it should.

This reverts https://crrev.com/c/1269235 for the case of a flex item
which is a flexbox with percentage children.

Bug: 927066
Change-Id: I955bf651f75495122bb454d5221303b188bbb03c
Reviewed-on: https://chromium-review.googlesource.com/c/1452477
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#628806}(cherry picked from commit 4f73107eed2d8f61e189dcd52239159129961750)
Reviewed-on: https://chromium-review.googlesource.com/c/1459323
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#280}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}

Comment 102 by bugdroid, Feb 7

Project Member
Labels: merge-merged-3683
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7fa7ef3bfd087d66ddfb7cbd8ad7297ef8b09b51

commit 7fa7ef3bfd087d66ddfb7cbd8ad7297ef8b09b51
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Thu Feb 07 19:33:09 2019

[css-flexbox] Don't apply min-height: auto to children with percentage heights

In such a case, we compute an incorrect intrinsic logical height, because
we resolve pre-flex percentages when we calculate the intrinsic height.
In common cases this can be 100% of the flexbox height, which we later
use as a min-height of the flex item, leading to it not shrinking when
it should.

This reverts https://crrev.com/c/1269235 for the case of a flex item
which is a flexbox with percentage children.

Bug: 927066
Change-Id: I955bf651f75495122bb454d5221303b188bbb03c
Reviewed-on: https://chromium-review.googlesource.com/c/1452477
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#628806}(cherry picked from commit 4f73107eed2d8f61e189dcd52239159129961750)
Reviewed-on: https://chromium-review.googlesource.com/c/1459323
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#280}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}
[modify] https://crrev.com/7fa7ef3bfd087d66ddfb7cbd8ad7297ef8b09b51/third_party/blink/renderer/core/layout/flexible_box_algorithm.cc
[add] https://crrev.com/7fa7ef3bfd087d66ddfb7cbd8ad7297ef8b09b51/third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-minimum-height-flex-items-012.html

Comment 103 by benmason@chromium.org, Feb 8

Cc: benmason@chromium.org

Comment 104 by dneelame...@chromium.org, Feb 8

Verified/Fixed on Asus Zenphone/5.0 and Pixel C/OPM8.190205.001 with M73/73.0.3683.27 webview

Comment 105 by abdulsyed@google.com, Feb 12

Labels: -M-72 -Target-72

Comment 106 by cbiesin...@chromium.org, Feb 12

yapaarachchi
> Is this, "min-height: 0" solution will work for all scenarios. Because,
> I found that it doesn't work for some cases

I am definitely interested in hearing more about this, because I would definitely expect this to work in all cases!

Comment 107 by copycat...@gmail.com, Feb 12

Hi

 Thanks for the hard work. We have several applications affected by this problem and the "min-height: 0" solution did not work for us. 

 When do you plan to release a fix for this?

Comment 108 by cbiesin...@chromium.org, Feb 12

copycat/anyone else who finds that min-height:0 did not fix the issue--

Could you send me more information, ideally a URL or code snippet showing that? That may be a different underlying cause. You can email me or file a new bug at crbug.com/wizard; please mention the bug# here if you file a new bug.

Comment 109 by a.fer...@gmail.com, Feb 13

I may be wrong, but seems to me Chrome 72 handles differently also the _width_  of the flex items when the container is "flex-direction: column;". The left and right margins of the flex items are just ignored. The flex items are always stretched to the max available width. To my understanding, left and right margins should be honored even if the default value of "align-items" is "stretch".

I can see this happening on one of our apps, where everything worked fine till Chrome 71 and still works fine with latest Safari, Opera, Firefox.

Can someone please confirm also the width in "flex-direction: column;" is affected?

Comment 110 by a.fer...@gmail.com, Feb 13

To clarify my previous comment: left and right margins are actually honored. However, the flex items have always a width "auto" so they actually overflow the flex container.

Comment 111 by cbiesin...@chromium.org, Feb 13 (6 days ago)

a.fercia, I'm not sure I understand your issue. http://plexode.com/eval3/#s=aekVQXANJVQMbAx1yAXgePQN5GwFHTUZZHAGeoFkORUpTRkRVSlBPnURQTVZOT6OLnatFo4KdExERUaKaHz1PyKSVl5kDhp0Sw1kBwcS5nUirRrjGNaFV4eLj5OIB5eTIHRByHwNeAA== seems to display correctly.

Do you have some example HTML showing it not working? Could possibly be related to https://chromium-review.googlesource.com/c/1327746 (a different root cause)

Comment 112 by cbiesin...@chromium.org, Feb 13 (6 days ago)

Cc: dholb...@gmail.com
 Issue 915936  has been merged into this issue.

Comment 113 by cbiesin...@chromium.org, Feb 14 (5 days ago)

I just checked in another fix for a related issue; if you update canary to 74.0.3705.0 or later, can you see if your sites work now? (Again, this only applies for cases where Chrome is rendering different than Firefox)

Comment 114 by kgre...@gmail.com, Feb 15 (5 days ago)

An update between versions 71.0.3578.98 and 72.0.3626.109 has caused the attached example to render incorrectly.  (A colleague of mine put this simple reproduction together, thanks Dylan Wulf!)

`.flex-container` uses `align-items: center` and `flex-wrap: wrap`

Disabling either of those restores it to the intended output.  

This is not fixed in canary 74.0.3706.0.
index.html
788 bytes View Download

Comment 115 by rmich...@edgeofthenet.org, Feb 17 (3 days ago)

I have a nested flexbox layout, quite a few levels deep, which appears to be affected by this issue.

The innermost flex div contains a block div, which contains only a span.

It has rendered properly on Chrome for at least the past year.  Chrome 72 has broken the layout of this page.  Regardless of the style of the span, the content height of the parent div is always 0.  When I change the outer flex container to display block, the block div has the correct implicit content height.

The layout is broken on:
  Version 72.0.3626.109 (Official Build) (64-bit)

The layout is again fine on beta or canary:
  Version 73.0.3683.39 (Official Build) beta (64-bit)
  Version 74.0.3708.0  (Official Build) canary (64-bit)

All current versions of Firefox (stable, dev, nightly) also render this page properly.

I have tried setting min-height and flex basis, on various elements, as suggested in comments here, but cannot seem to work-around the problem.

The layout is not terribly complicated, however, something odd must be happening as I am finding it very challenging to create a simple reproducer. Although I realize it will not be very helpful without the complete DOM and style information, I've attached screenshots to show the zero parent content height I described above while I continue to try to find a reproducer.

I'd appreciate suggestions on a reproducer or work-around.
72.0.3626.109-parent-content-height-is-zero.png
141 KB View Download
72.0.3626.109-child-content-height-is-auto.png
148 KB View Download
72.0.3626.109-child-content-height-is-explicit-parent-still-zero.png
138 KB View Download
74.0.3708.0-parent-has-content-height.png
90.8 KB View Download
74.0.3708.0-child-content-height-is-auto.png
99 KB View Download
firefox-67-nightly-to-show-flex-ancestors.png
87.6 KB View Download

Comment 116 by a.fer...@gmail.com, Yesterday (26 hours ago)

cbiesin... sorry for late reply, I didn't get a notification.

See this example, which reproduces a case we have in production:
https://codepen.io/afercia/full/RvdPMy

this renders differently on Firefox and worked fine till Chrome 71.
Showing comments 17 - 116 of 116 Older

Sign in to add a comment