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

Issue 890100 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Sep 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Wallpaper picker loses the top header

Project Member Reported by wzang@chromium.org, Sep 28

Issue description

On ToT, the wallpaper picker (which can be opened from right clicking the background) loses its top header. The height is 0, although in css it's specifies as 40px.

The code point is at:
https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/wallpaper_manager/css/wallpaper_manager.css?q=top-header&dr=C&l=406
 
Screenshot from 2018-09-27 17-00-14.png
1.6 MB View Download
Screenshot from 2018-09-27 17-03-12.png
1.6 MB View Download
Cc: cbiesin...@chromium.org omrilio@chromium.org elizabethchiu@chromium.org
cbiesinger@, bisect shows this is a regression caused by https://chromium-review.googlesource.com/c/chromium/src/+/1246730

I'm happy to change the Chrome OS wallpaper picker code to accommodate your change.

But for the time being I think it makes sense to revert it because:
1) It may have silently broken other UI.
2) This is directly user-facing on Chrome OS.

Could you please fix this before relanding your CL? Thanks!
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28

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

commit f033b7d2fc1c1a44006a9c30afc864abc9e7571b
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Fri Sep 28 21:51:14 2018

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

To avoid the previous regression (see analysis in bug), we force
layout in the case where we otherwise would get an outdated result.

This relands https://chromium-review.googlesource.com/c/chromium/src/+/1246730
with an additional fix for the Chrome UI CSS. I also split out the
test change into its own patch.

IF THIS BREAKS ANY FURTHER CHROME UI:
Please don't revert this patch; instead, add min-height: 0 to any
inner nested flexboxes that may be affected by this patch. This is
an important change for web interop with the other browsers.

Bug:  596743 , 890100 
Change-Id: Ice629c2a7823ef07d075fa23b99022b4012c6200
Reviewed-on: https://chromium-review.googlesource.com/1252682
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595225}
[modify] https://crrev.com/f033b7d2fc1c1a44006a9c30afc864abc9e7571b/chrome/browser/resources/chromeos/wallpaper_manager/css/wallpaper_manager.css
[modify] https://crrev.com/f033b7d2fc1c1a44006a9c30afc864abc9e7571b/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/f033b7d2fc1c1a44006a9c30afc864abc9e7571b/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/f033b7d2fc1c1a44006a9c30afc864abc9e7571b/third_party/blink/renderer/core/layout/layout_flexible_box.h

Status: Fixed (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 4

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

commit 58266dadab879824045b0a24816fe30fbb366b34
Author: Wenzhao (Colin) Zang <wzang@chromium.org>
Date: Thu Oct 04 17:04:22 2018

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

This reverts commit f033b7d2fc1c1a44006a9c30afc864abc9e7571b.

Reason for revert:  crbug.com/891988 

Original change's description:
> [css-flexbox] Apply min-height: auto to nested flexboxes again
> 
> To avoid the previous regression (see analysis in bug), we force
> layout in the case where we otherwise would get an outdated result.
> 
> This relands https://chromium-review.googlesource.com/c/chromium/src/+/1246730
> with an additional fix for the Chrome UI CSS. I also split out the
> test change into its own patch.
> 
> IF THIS BREAKS ANY FURTHER CHROME UI:
> Please don't revert this patch; instead, add min-height: 0 to any
> inner nested flexboxes that may be affected by this patch. This is
> an important change for web interop with the other browsers.
> 
> Bug:  596743 , 890100 
> Change-Id: Ice629c2a7823ef07d075fa23b99022b4012c6200
> Reviewed-on: https://chromium-review.googlesource.com/1252682
> Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595225}

TBR=cbiesinger@chromium.org,eae@chromium.org,wzang@chromium.org,mstensho@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  596743 ,  890100 
Change-Id: I8b3ce623a49fe476dbc309b9d780fee80b233c3e
Reviewed-on: https://chromium-review.googlesource.com/c/1261919
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596719}
[modify] https://crrev.com/58266dadab879824045b0a24816fe30fbb366b34/chrome/browser/resources/chromeos/wallpaper_manager/css/wallpaper_manager.css
[modify] https://crrev.com/58266dadab879824045b0a24816fe30fbb366b34/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/58266dadab879824045b0a24816fe30fbb366b34/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/58266dadab879824045b0a24816fe30fbb366b34/third_party/blink/renderer/core/layout/layout_flexible_box.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 16

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

Sign in to add a comment