Note this also affects functionality because the buttons in the bottom are unclickable. Users can't add other WiFi networks.

cbiesinger@: Bisect shows https://chromium-review.googlesource.com/c/chromium/src/+/1252682 is the culprit (again), but it can't be reverted this time. It seems that the test portion of the CL was separated and in the CL description it says:

"...so that it won't be reverted in case the patch reland
will be reverted again."


Please fix this ASAP. I think it should be escalated to P0 later if remaining unfixed.

IMO, adding "min-height: 0" is not a good solution because:
1) It feels hacky,
2) We don't have the resources to examine each and every affected case, especially one week before branch.
3) Despite a PSA was sent, how would an engineer in the future (or even one month later) know this must be added when working on new UI?

I'll be on an airplane most of the day... where does this code live?

I'm not sure why you say it feels hacky. This bug happens because the implicit min-height: auto is bigger than what the specific UI intends. It is what the Flexbox spec requires and what other browsers implement already.

Also, we already had this behavior when the child wasn't a flexbox (even if it is a regular div containing a flexbox)

The code lives in https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/?type=cs&q=cr-network-select+file:%5Esrc/chrome/browser/resources/chromeos/login/+package:%5Echromium$&g=0

Sorry, it's not fair to say it feels hacky: we could add "min-height: 0", but I'm wondering:

1) Is there a more automatic way to find nested flexboxes? Also, can we add tests to catch such bugs earlier?

2) Your CL has perf impact. Why can't we allow more time to think of a fix to reduce the perf impact, while we have enough time to make changes on our side?

Re your second point: I've been thinking about a better way to fix this since https://chromium.googlesource.com/chromium/src/+/b883e94b36fe4f3f3f71e24626ed9e0678372085 and have not thought of one; it seems unlikely that one will come to me in the next few weeks.

Do you think reverting the patch would make it *easier* for you to find all the places that need changes? The fixes themselves are pretty easy...

Re 1: I guess we could apply min-height: 0 to all flexboxes in the UI; that change should be safe in that it will keep the current behaviour in all cases (I guess except when that might override another explicit min-height). I can't think of a way to find only all nested flexboes; even less the nested flexboxes where this did cause a behavior change.

There probably are ways to write tests (check the rectangle of the buttons to make sure they're within the rectangle of the parent, or something). Not sure how valuable they are, but maybe.

Do you have any guess on why this only caused regressions on ChromeOS so far, not in the rest of the Chrome UI? (Maybe it just uses HTML more?)

Also, it seems you have a good handle on where this specific bug could be fixed, do you still want me to take a look? I'm about to take off on a transatlantic flight and will not be able to look at this quickly.

(by the way, this change *can* be reverted, I would just prefer not to. I'm not sure why you said it can't be. That the test landed separately shouldn't matter, reverting the patch should just mark it as failing, which is fine)

(cc eae, fyi)

Please provide detail on the unclickable buttons.  Will a user be able to make a network selection at all,or would the device essentially be bricked due to this behavior?  I need to determine soon for release planning.

If this is strictly cosmetic, or if there's a way to mitigate I'm going to block beta rather than dev since there are some time constraints.  Thx

Re #6, thanks, hoped to sync with you before reverting it, especially when the CL description says not to.

We'll apply "min-height: 0" in OOBE, in the meantime this patch should be reverted for now. It can be relanded right after M71 branch next Thursday. By that time we should finish the OOBE changes, and it will allow another milestone to find out other UI which is silently broken (there's very likely to be more).

Re #7, removed the tag since the culprit CL is reverted.

CL may have been reverted but it's still going to be in today's DEV candidate.

Can I get some response to #7?  Fairly critical.  Thanks

Re #11, the first several (~7) networks can still be selected, but not those that overflow to the bottom (those that would have been hidden without the bug)
So it depends on how long the available network list is. Also  "Proxy Settings" and "Add other WiFi network" are often non-clickable because they always come the last.

But, I think users can work around this by selecting network from the system tray (if they only want to select a network or add new network. Not sure if we have the support for vpn and proxy settings from the system try.)

 Issue 890662  has been merged into this issue.

This issue is fixed after reverting https://chromium-review.googlesource.com/c/chromium/src/+/1252682.

Will file a new bug to migrate OOBE to work with the nested flexboxes updates because the CL will be relanded in M72.

The network page is still messed up while adding new user on Nocturne - R71- 11150.0.0, 71.0.3575.1. 
Please see attached screenshot.

Deleted: nocturne-networks.jpeg 515 KB

	nocturne-networks.jpeg 515 KB View Download

Re c#19: This is a different issue. This issue is only reproducible when there're extra texts at top, but NOT on a regular error screen in c#0.

zalcorn@, elizabethchiu@, per offline chat with alemate@, we have two options:

1) Reduce the height of the network drop down list, so that the bottom two lines of texts will move up. However, the drop down will be very narrow, and probably can only show one network entry at a time.

2) Make all the contents scrollable, which is easier to implement, but users have to scroll down to the bottom in order to see the "browser as guest" and "sign in as existing user" options.

Any ideas? 

The original issue in c#0 is fixed. Filing  crbug.com/895000  to track the issue in c#19.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef298ae538d3b965b036fe68ace5592841cf15ae

commit ef298ae538d3b965b036fe68ace5592841cf15ae
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Tue Oct 16 03:20:01 2018

Add min-height: 0 to ChromeOS UI flexboxes

This is required before (re)landing a Flexbox change ( bug 596743  /
https://chromium-review.googlesource.com/c/chromium/src/+/1269235)

This fixes the previously found issues in the wallpaper manager and
the OOBE UI. This changes more than necessary, but this is easier
and should not be harmful.

Bug:  890100 ,  891988 ,  892757 
Change-Id: I745a22fadabd182d06d6b4c67b428d495c48bae4
Reviewed-on: https://chromium-review.googlesource.com/c/1281726
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599830}
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/api_keys_notice.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/discover/discover_app.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/discover/discover_card.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/enterprise_card_footer.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/gaia_card.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/gaia_header.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/md_top_header_bar.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/multidevice_setup_first_run.html
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/network_select_login.html
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/offline_gaia.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_a11y_option.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_change_picture.html
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_dialog_host.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_flex_layout.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_popup_overlay.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_auto_enrollment_check.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_enable_debugging.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_reset.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_terms_of_service.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_voice_interaction_value_prop.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/oobe_screen_wait_for_container_ready.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/screen_arc_kiosk_splash.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/screen_arc_terms_of_service.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/screen_device_disabled.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/screen_gaia_signin.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/login/throbber_notice.css
[modify] https://crrev.com/ef298ae538d3b965b036fe68ace5592841cf15ae/chrome/browser/resources/chromeos/wallpaper_manager/css/wallpaper_manager.css