MacViewsBrowser: Omnibox popup has 2 shadows |
|||||
Issue description
Chrome Version : 63.0.3236.0
OS Version: OS X 10.12.6
with gn args, mac_views_browser = true. See attached.
It's hard to see in the screenshot, but the shadow on the left side is slightly darker where it's next to the omnibox dropdown, than where it's on the browser only. This is because both the browser window and the omnibox popup shadows are being composited on top of each other.
The popup has a shadow because Widget::InitParams::TYPE_POPUP currently adds a shadow on MacViews. On other platforms, it seems not to, but I haven't figured out where that logic is yet, so we can share it. (Maybe it is somehow avoided because these Widgets are not "toplevel").
The fix is either to call NSWindow setHadShadow:NO for TYPE_POPUP, or set InitParams::SHADOW_TYPE_NONE in omnibox_popup_contents_view.cc
The first option is probably better but affects other widgets (e.g. fullscreen's `SubtleNotificationView`) so we need to be sure they don't regress. And ideally, it shares the logic for `func(InitParams::type, SHADOW_TYPE_DEFAULT) -> {SHADOW_TYPE_XXX}` amongst all platforms.
,
Oct 26 2017
I'm not seeing it on 64.0.3250.0 (10.12.6)
,
Oct 27 2017
Yup - this doesn't happen in official releases -- just when building with mac_views_browser = true in gn args. But thanks for the pointer! That's interesting - I toyed with omnibox::kUIExperimentNarrowDropdown and couldn't actually get a narrow dropdown. It does look like we want to do some funky stuff with window shadows... which do not animate great on Mac.
,
Apr 2 2018
MacViews triage: to lgrey@ for M68.
,
Apr 3 2018
The default omnibox shadow for chrome://flags/#top-chrome-md = Refresh is no longer correct for Windows either. Tommy has been poking this in https://chromium-review.googlesource.com/c/chromium/src/+/974501 . We should be able to get the solution there working for Mac as well. Then we just need to suppress the WindowServer-drawn shadow using Widget::InitParams::shadow_type
,
Apr 13 2018
,
Apr 25 2018
Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
,
Apr 25 2018
It's quite possible doing this CL for Mac as well will fix the issue: https://chromium-review.googlesource.com/c/chromium/src/+/1028833 Also see how it looks like when --top-chrome-md=material-refresh
,
Apr 25 2018
I think it's a non-issue with --top-chrome-md=material-refresh since the omnibox isn't flush with the window edge. There's the weird inner shadow (see attached), but I don't think it's quite the same bug.
,
May 29 2018
I'm closing this since c#9 is a different issue and the MD refresh omnibox popup shouldn't ever be flush with the browser window. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by k...@chromium.org
, Oct 26 2017