Skip to content
Commit a597da3c authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot
Browse files

macviews: add focus ring to omnibox

This change is somewhat complicated by the fact that the omnibox gets
focus, but the location bar (which contains it) should get the actual
ring. Therefore:

1) Add LocationBarView::OnOmnibox{Focused,Blurred} to notify the
   location bar that the omnibox gained/lost focus;
2) Have OmniboxViewViews call LocationBarView::OnOmnibox{Focused,
   Blurred} at appropriate times;
3) Add ChromePlatformStyle to hold per-platform UI differences, by
   analogy with //ui/views PlatformStyle;
4) Add ChromePlatformStyle::kOmniboxUsesFocusRing to specialize
   this behavior for Mac;
5) Export FocusRing from Views for use here;
6) Have LocationBarView install/uninstall a focus ring as needed to
   track omnibox focus state;
7) Have LocationBarView::Layout() call View::Layout(), so that the
   focus ring's layout gets updated when the LocationBarView changes
   size;
8) Stop calling SetMasksToBounds(true) in LocationBarView::Init(),
   since this prevents the focus ring from drawing outside the
   LocationBarView's bounds, which is exactly where focus rings are
   supposed to draw.

Two concerns:
It seems that (7) might cause redundant work with the rest of
LocationBarView::Layout(), since it will lay out all the subviews,
but since LocationBarView has no LayoutManager I think this is
actually more or less a no-op.
The layer masking at (8) was originally added to fix some bugs (like
https://crbug.com/588331) where children of LocationBarView were
unintentionally drawing outside its bounds. I tried to reproduce these
bugs without the layer masking, and they don't show up locally, so
perhaps they were fixed elsewhere.

Bug: 826270
Change-Id: Ic7dd287d0260cfce0f3a00680027aaa2d87fdcb1
Reviewed-on: https://chromium-review.googlesource.com/980679


Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546882}
parent 9f7dd1c0
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment