New issue
Advanced search Search tips

Issue 836946 link

Starred by 5 users

Issue metadata

Status: Duplicate
Merged: issue 851122
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

[macviews] omnibox focus ring too thick

Project Member Reported by thakis@chromium.org, Apr 25 2018

Issue description

(compare to regular chrome)
 
Screen Shot 2018-04-25 at 4.10.49 PM.png
6.4 KB View Download

Comment 1 by thakis@chromium.org, Apr 25 2018

Summary: [macviews] omnibox focus ring too thick (was: [macviews] omnibox focus ring too think)

Comment 2 by meh...@chromium.org, Apr 25 2018

Components: UI>Browser Internals>Views>Desktop
Labels: Proj-MacViews
Labels: -Pri-3 MacViews-Controls M-68 Target-68 Pri-2
Owner: ellyjo...@chromium.org
Status: Assigned (was: Untriaged)
Pixel-by-pixel of the omnibox focus rings:

Cocoa Chrome: outer 1px ring of A0CAFB, middle 1px ring of 7DA7D7, inner 1px ring of A1CBFB
Views Chrome: outer *2px* ring of ACCEF4, middle 1px ring of 80A2C9, inner 1px ring of B3D5FC
Safari: outermost 1px ring of A4BFEB, then 1px of 99C0EB, then 1px of 8FB6E1, then innermost 1px of A5CCF6

The Views one is quite close to the Safari one color-wise. That said, I do like the look of the Cocoa Chrome ring more, so I will tweak ours to be a bit brighter and thinner.
Labels: Sprint-2
Any progress here?
Project Member

Comment 6 by bugdroid1@chromium.org, May 21 2018

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

commit 824d232c3d3630d9486a33f3cbc9be6077654c40
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Mon May 21 16:04:51 2018

views: introduce focus ring paths

This change adds support for Mac-style "focus ring paths" to Views, which
means that a given View can (via FocusRing::Delegate) supply an arbitrary path for its focus ring to
be drawn around when it has a focus ring. This removes the need for Checkbox
and RadioButton to have their own focus ring implementations. It will also
be necessary as the MD refresh work adds more circular/oval controls in
top chrome.

This work leads into a longer-term effort to make FocusRing not be a View;
see https://crbug.com/840796 for details about that.

Bug:  836946 , 835376 
Change-Id: I433dc370e5fbc2ff30b53b070d84255199b6ec29
Reviewed-on: https://chromium-review.googlesource.com/1050185
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560278}
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/frame/app_menu_button.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/page_action/page_action_icon_view.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/page_action/page_action_icon_view.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/chrome/browser/ui/views/toolbar/toolbar_button.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/button.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/button.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/checkbox.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/checkbox.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/md_text_button.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/md_text_button.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/radio_button.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/button/radio_button.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/combobox/combobox.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/combobox/combobox.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/focus_ring.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/focus_ring.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/scroll_view.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/controls/textfield/textfield.h
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/examples/button_sticker_sheet.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/view.cc
[modify] https://crrev.com/824d232c3d3630d9486a33f3cbc9be6077654c40/ui/views/view_observer.h

Cc: phanindra.mandapaka@chromium.org
Labels: Needs-Feedback
Able to reproduce the issue on chrome version 68.0.3406.0(build without fix)

Verified the fix on Mac 10.13.3 Chrome version #68.0.3437.2 as per the comment #0.

Attaching screeshots for reference.

could you please take a look at screenshot and help us in verifying the fix.

Thanks...!
836946.png
177 KB View Download
Status: Started (was: Assigned)
#7: is that macviews browser? This bug only affects macviews browser, and isn't fixed yet - the CL in #6 is infrastructure only.
Labels: -Target-68 Target-69
Mergedinto: 851122
Status: Duplicate (was: Started)
Labels: -M-68 Group-Focus_Input_Selection_Activation_KeyState

Sign in to add a comment