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

Issue 678510 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression: Overlapping of hover effects for icons in omnibox is observed.

Reported by jshan...@etouch.net, Jan 5 2017

Issue description

Chrome Version: 57.0.2972.0 (Official Build) b1f498915edb2115c8f993e60e19728afda80acb-refs/heads/master@{#441559}-32/64 bit
OS: Windows(7,8,8.1,10),Linux (14.04 LTS)

Steps:
1. Launch Chrome, open NTP and zoom in/out page.
2. Double click on "Zoom" icon in omnibox, hover mouse on "Star icon" and observe.

Actual: Overlapping of hover effects for icons in omnibox is observed.

Expected: No such overlapping for icons in omnibox should be seen

This is a regression issue broken in M-53, will soon update the other info:

Good Build: 53.0.2784.0
Bad Build: 53.0.2785.0

Note: Above issue is not seen for Mac OS.
 
Actual_result.jpg
31.1 KB View Download
Labels: hasbisect-per-revision
Owner: est...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 53.0.2784.0 (Revision: 403038).
Bad build: 53.0.2785.0 (Revision: 403382).

You are probably looking for a change made after 403169 (known good), but no later than 403170 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/e959e110424ef144189fa2046e046f6a5003d5ee..fc47fab1198649e7e3c7e447db6453ae7f2ba612

@estade -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank You.
Cc: bruthig@chromium.org est...@chromium.org
Labels: OS-Chrome
Owner: sgabr...@chromium.org
hmm, yea that does look bad. I'm not sure what the desired behavior is. I think the hover effect and "active" effect are both correct, they just happen to overlap. What ought we to be doing? Over to Sebastien for ideas.
Cc: bettes@chromium.org
+bettes.
Did we reduce the space between omnibox icons ?
The fix is easy, push the icon apart so that the active/hover states don't overlap. Add 2px of padding for balance.
Owner: bettes@chromium.org
Cc: maxwalker@chromium.org
Adding mac just in case it's part of his omnibox work.
I think the hover effect should fill the actual hover area.  Right now the effect seems to overshoot the actual hover area, which is misleading.  Compare this to, say, the toolbar buttons, where the hover effect does not overshoot the hover area (in fact, if anything it undershoots by 1 or 2 px).

I'm guessing this was done because the real hover areas for these icons are not currently square.  If we really want them to be square, we will need to make these views wider, which is going to make the icons feel very spread out if there are multiple visible icons.  Or we can just leave the current padding and have rectangular hover effects instead of square ones.
Owner: est...@chromium.org
Sebastien's right, except it's 1dp/pt between hover states and omnibox. See attachment. 

For Windows/cros/linux:
- move bookmarks 1px to the right (1dp/pt from the omnibox edge). Screenshot in #1 has uneven padding.
- position subsequent icons 1dp/pt apart

For Mac:
- position subsequent icons 1dp/pt apart. Currently there is 0 pixels between each ( crbug.com/678830 )

Artboard.png
47.7 KB View Download
@7: That doesn't account for the fact that, at least on Windows, the actual hover/activation areas don't actually match the areas the graphical effect covers.  (i.e. the first paragraph of comment 6)

On views, there is a several-pixel-wide dead zone between icons currently.  I don't remember whether we did this intentionally for touch targeting.  It seems like that should be removed as part of fixing this.
Labels: -Pri-1 Pri-2
if anyone wants to take this bug before I mark it started, feel free. It seems definitely not a p1 to me.
Cc: sgabr...@chromium.org
We also need to worry about what it should look like in hybrid (touch) mode.
wrt event targets, I think we can/should make the icons bigger (i.e. match the hover effect, which is 24x24, instead of what we're apparently using, which is 16x24). I think current behavior is probably an oversight.
Owner: bettes@chromium.org
> We also need to worry about what it should look like in hybrid (touch) mode.

And by "we", I mainly meant designers :) The difference in touch mode is that the omnibox is 4 dip taller (32 instead of 28).
Owner: est...@chromium.org
The regression is about overlapping hovers so can we fix that first with the suggested fix in #7 and then file a separate bug for the following? 

1.) the size of the hover and hover effect for all platforms
2.) what to do on touch mode, assuming that overlapping isn't an issue there? 

Owner: bettes@chromium.org
How it looks in touch mode affects how we fix non-touch mode because the code is all inter-related. I don't think it's possible to fix one mode without affecting the other, and while don't believe this is a huge priority to fix in any mode, I do think we should fix it all at once.

re: (1) you've already specified the size of the hover effect, and I see no reason not to make the event target match that.
Please position the icons 1pt apart in non-touch mode (as described by Alan) and 3pt apart in hybrid mode so that the spacing between the icons and the omnibox border is visually balanced. Hover/target areas should be 24x24pt in both cases. Thanks!
Omnibox Icons.png
328 KB View Download
thanks.
Owner: est...@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 26 2017

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

commit 39f5ad11373872bbb7e398fb7943b289cc790c39
Author: estade <estade@chromium.org>
Date: Thu Jan 26 01:45:17 2017

Adjust positioning of location bar icons.

See bug for new layout spec. Note that this affects not just the
trailing icons but also the location icon ,which is now slightly
closer to the left edge of the location bar.

Test cases:
- location icon, no text (such as an http:// site)
- location icon w/text (such as an https:// site)
- alignment of contents of omnibox popup, both icons and text, with contents of location bar
- keyword hint view, i.e. text on right of location bar when typing a searchable keyword
- selected keyword view, i.e. after pressing tab to search
- content settings icon, e.g. blocked microphone on permission.site
- content settings icon w/text, e.g. a blocked popup on popuptest.com
- multiple location icons, like star view + zoom view
- all of the above in touch mode (--top-chrome-md=material-hybrid)

BUG= 678510 

Review-Url: https://codereview.chromium.org/2642893002
Cr-Commit-Position: refs/heads/master@{#446199}

[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/layout_constants.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/layout_constants.h
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/39f5ad11373872bbb7e398fb7943b289cc790c39/chrome/browser/ui/views/omnibox/omnibox_result_view.cc

Status: Fixed (was: Started)

Sign in to add a comment