New issue
Advanced search Search tips

Issue 856755 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

If suggestion has favicon, match it in the omnibox

Project Member Reported by emilyschechter@chromium.org, Jun 26 2018

Issue description

If a suggestion has favicon, match it in the omnibox. 
 
Cc: tommycli@chromium.org jdonnelly@chromium.org
Owner: manukh@chromium.org
See the attached image.

If the highlighted suggestion has a favicon, make the icon in the Omnibox itself match the suggestion rather than show a default globe.

chrome://flags/#omnibox-ui-show-suggestion-favicons must be on, of course.
image (1).png
52.8 KB View Download

Comment 2 by manukh@chromium.org, Jun 27 2018

Status: Started (was: Untriaged)

Comment 3 by manukh@chromium.org, Jun 28 2018

emilyschechter@, just to confirm, this is just for favicons, and not other icons? For example, If I bookmark a website that has no favicon (e.g. example.com), the omnibox suggests will display a star next to that result, but the omnibox top should still show the globe it shows today?
Cc: bklmn@chromium.org
Interesting... it seems reasonable to me to match whatever the icon is, including bookmark. Are there any other cases like that you can think of?

cc'ing Joel in case he has an opinion.

Comment 5 by manukh@chromium.org, Jun 28 2018

emilyschechter@ 
The search queries? For example, in Tommy's screenshot above, the 4th suggestion is a google search for the term 'facebook. In current stable, both the suggestion and the omnibox show a magnifying glass. In current material design, the suggestion shows the chrome icon (as in the screenshot) and the omnibox shows the search provider icon (e.g. `G` for google) or defaults to the chrome icon if the search provider isn't google. I feel like this is this intentional / correct behavior, but want to make sure.
Screenshot from 2018-06-28 12-25-46.png
15.0 KB View Download
Screenshot from 2018-06-28 12-27-02.png
28.3 KB View Download
Screenshot from 2018-06-28 12-27-52.png
29.8 KB View Download

Comment 6 by manukh@chromium.org, Jun 28 2018

clarification on above comment: current material design will use the generic search icon (which looks like a circle around a dot in the suggestions, but will display the search provider's icon in the omnibox. If the search provider has no icon provided, then the omnibox too will default to the circle & dot icon.

Should the suggestions also use the search provider's icon for search suggestions (and thereby match the omnibox icon) or should they remain different?
Good question but yes, we deliberately choose to not use the search provider's favicon for search suggestions. We might revisit that later, though.

Agreed that it would make sense to mirror the bookmark icon in the omnibox, if that's not especially difficult.
I think we should limit this to having Refresh enabled as wells as having favicons enabled. My reasoning being that we only show search engine favicons in Refresh and we should do the same for pages.
Double checking,

For the website favicon:
Both the omnibox and the popup will check both flags (refresh & favicon).
(Currently, the omnibox never shows a favicon; the popup checks only the favicon flag)

For the search provider icon:
The omnibox will check the refresh flag. The popup will always show the magnifying glass icon.
(Currently, same behavior)

For the bookmark star icon
Both the omnibox and popup will show the bookmark icon regardless of either flag.
(Currently, the omnibox never shows the bookmark icon regardless of either flag; the popup always shows the bookmark icon regardless of either flag)
#9: everything there is correct with one exception. The omnibox should only show the bookmark icon if the refresh flag is enabled.

Noted just for the record here, manukh already made this adjustment in his WIP CL: https://crrev.com/c/1124671.
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 16

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

commit 6291a1f17a1f81dc42efe96e5db0decf04dd4d13
Author: manuk <manukh@chromium.org>
Date: Mon Jul 16 20:12:15 2018

Show favicon and bookmark star in omnibox.

When the user activates the omnibox popup, the popup suggestions have favicons and bookmark stars when appropriate. Selecting one (either because one is selected by default or by using the arrow or tab keys), displays the suggested match in the omnibox textfield. Previously, when the selected suggestion had a favicon or bookmark star icon, the omnibox would display the url or search icons. There is still a discpreency between omnibox and omnibox popup suggestion icons; with material refresh active, search suggestion icons will display the default search icon (magnifying glass) whereas the omnibox will display the default search provider's icon if available (`G` for Google).

Bug: 856755
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I5874d72d0bde711fb3870a1ab0fd1f6e5c699eb8
Reviewed-on: https://chromium-review.googlesource.com/1124671
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575400}
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/omnibox_view.cc
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/omnibox_view.h
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/omnibox_view_unittest.cc
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/test_omnibox_client.cc
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/test_omnibox_client.h
[add] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/test_omnibox_edit_model.cc
[add] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/test_omnibox_edit_model.h
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/test_omnibox_view.cc
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/omnibox/browser/test_omnibox_view.h
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/components/toolbar/toolbar_model.h
[modify] https://crrev.com/6291a1f17a1f81dc42efe96e5db0decf04dd4d13/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm

Cc: manukh@chromium.org
Labels: -Pri-2 Pri-3
Owner: emilyschechter@chromium.org
Verified in Canary. Assigning to emilyschechter to confirm that it's working as intended.
Suggestion: If omnibox is in edit mode and suggestion box is not activated (i.e. click Omnibox twice), perhaps we shouldn't show the favicon?

Also, if you edit the Omnibox to a previously visited site, say google.com, and you un-focus the Omnibox by clicking on the page, then the favicon is still visible (Google favicon), but the lock icon disappears.

It feels weird to have these behaviours:

- If no changes were made to the Omnibox in edit mode, unfocusing Omnibox hides the favicon and restores the lock icon (assuming current site is secure)

- If changes were made to the Omnibox in edit mode, unfocusing Omnibox does *not* hide the favicon, and lock icon is gone

I think the lock icon should be shown in place of the favicon if the Omnibox is visible but the suggestion dialog is not.

Sign in to add a comment