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

Issue 780835 link

Starred by 64 users

Issue metadata

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

Blocked on:
issue 851015



Sign in to add a comment

Tab switch suggestions

Project Member Reported by k...@chromium.org, Nov 2 2017

Issue description

Feature description: Suggest open tabs in Omnibox results, and facilitate switching to them, instead of navigating to the site, to reduce tab usage/proliferation.

Eng owner: krb@chromium.org
Product owner:

Design doc: <see http://go/DesignDocTemplates>

Are you planning on experimenting before launch?
Any new strings?
Any implications for Google webservices (i.e. sync, translate)?
Binary size?
Do the existing perf tests exercise all aspects of your new feature(s)?

 

Comment 1 by k...@chromium.org, Nov 2 2017

Here are some TODOs that I don't want to lose:

- use in other providers
- proper logging proto type
- i18n string when UI settled
- navigate option (i.e., don't switch to tab)
- control-clicking to open in background tab (what should this do?)
- make sure that it does not allow searching/switching between incognito
windows (unless in the same profile)
- decide whether switch-to-tabs should be recorded in the shortcuts
  database.  (Current code does not.)
- avoid converting suggestions to 'this' tab
- move HQP conversion call into 'DoAutocomplete'
- handle more NTPs
- add testing
- create map from URL to WebContents

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 6 2017

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

commit 898669974e9ce0867e3dc80fe9385de8983a77ce
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Nov 06 15:34:49 2017

[omnibox] Include tab headings in suggestions and allow switching

Include tab URLs and titles in search suggestions in the Omnibox, and
allow the user to quickly switch to an already open tab.

Prevents searching/switching between different levels of anonymity
(incognito-ness).

Solely for experimentation currently.

TODO:

- use in other providers
- proper logging proto type
- i18n string when UI settled
- navigate option (i.e., don't switch to tab)
- control-clicking to open in background tab (what should this do?)
- make sure that it does not allow searching/switching between incognito
windows (unless in the same profile)
- decide whether switch-to-tabs should be recorded in the shortcuts
  database.  (Current code does not.)
- avoid converting suggestions to 'this' tab
- move HQP conversion call into 'DoAutocomplete'
- handle more NTPs
- add testing
- create map from URL to WebContents

(we'll add a more targetted bug too)

Bug: 780835
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ie06c87c415a9ee0ad801492e7515170d992df873
Reviewed-on: https://chromium-review.googlesource.com/692694
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514135}
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/about_flags.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/autocomplete/chrome_autocomplete_provider_client.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/ui/app_list/search/omnibox_result.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/autocomplete_match.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/autocomplete_match_type.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/autocomplete_match_type.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/autocomplete_provider_client.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/history_quick_provider.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/history_quick_provider.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/mock_autocomplete_provider_client.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/omnibox_edit_controller.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/omnibox_edit_controller.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/omnibox_field_trial.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/omnibox_metrics_provider.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/components/omnibox/browser/shortcuts_backend.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/ios/chrome/browser/ui/omnibox/omnibox_util.cc
[modify] https://crrev.com/898669974e9ce0867e3dc80fe9385de8983a77ce/tools/metrics/histograms/enums.xml

Comment 3 by k...@chromium.org, Nov 7 2017

Cc: emilyschechter@chromium.org mpear...@chromium.org
Another TODO:

If suggestion doesn't have a title, handle better.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 8 2017

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

commit 1f9f92864cc4b3cbcc57628244e508eca33c951c
Author: Kevin Bailey <krb@chromium.org>
Date: Wed Nov 08 15:29:25 2017

[omnibox] Tab switch suggestions improvements

3 small changes:
- i10n-alize hint string
- don't switch to 'this' tab
  still TODO: don't suggest 'this' tab
- push |ConvertOpenTabMatches()| to base class and
  call from HPQ and HUP

Bug: 780835
Change-Id: I16507d84e5ccedaae81028c49fd47e650ccda4ab
Reviewed-on: https://chromium-review.googlesource.com/755373
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514833}
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.cc
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/components/omnibox/browser/history_provider.cc
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/components/omnibox/browser/history_provider.h
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/components/omnibox/browser/history_quick_provider.cc
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/components/omnibox/browser/history_quick_provider.h
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/components/omnibox/browser/history_url_provider.cc
[modify] https://crrev.com/1f9f92864cc4b3cbcc57628244e508eca33c951c/components/omnibox_strings.grdp

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 13 2017

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

commit 98e533d332344ee7fdf2cace842c61b25b4a2020
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Nov 13 19:47:01 2017

[omnibox] Add an icon for tab switch suggestions

Add a new icon for tab switch suggestions, to visually distinguish them
from suggestions which navigate.

Bug: 780835
Change-Id: I6aac9379808da0d7217d9fe84350c48c25104ff6
Reviewed-on: https://chromium-review.googlesource.com/766447
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516011}
[modify] https://crrev.com/98e533d332344ee7fdf2cace842c61b25b4a2020/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/98e533d332344ee7fdf2cace842c61b25b4a2020/components/omnibox/browser/autocomplete_match.cc
[add] https://crrev.com/98e533d332344ee7fdf2cace842c61b25b4a2020/components/omnibox/browser/vector_icons/tab.icon

Comment 6 by meh...@chromium.org, Nov 16 2017

Hello krb@. I noticed that the icon seems to be 1px too large horizontally. 

Please find attached two screenshots: actual vs. expected.

On the actual screenshot you'll see that the icon is 1px too large on the left side.

This is on macOS (non-retina screen).
actual.png
15.9 KB View Download
expected.png
15.9 KB View Download
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 17 2017

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

commit 71f07f2c6c77c9e67158a193516e2f0c6229ce64
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Nov 17 02:18:21 2017

[omnibox] Adjust tab icon

Bring in left edge of tab switch suggestion icon.

Bug: 780835
Change-Id: Iec584365bdfcaadcc0d07b700297c190d5345628
Reviewed-on: https://chromium-review.googlesource.com/775029
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517250}
[modify] https://crrev.com/71f07f2c6c77c9e67158a193516e2f0c6229ce64/components/omnibox/browser/vector_icons/tab.icon

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 20 2017

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

commit d62befdc3f11dafaed46f023829dab4fd79a59d0
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Nov 20 22:12:33 2017

[omnibox] Handle empty descriptions in tab switch suggestions better

When a page doesn't have a title, the Omnibox can't give it a
description. If we blindly concat strings together, it looks odd.
This change makes a slight change for suggestions with empty
descriptions. Should only affect appearance.

Also adds a new small history_provider_unittest.cc

Bug: 780835
Change-Id: Ieb3f0911a01a1c1bb1b22013bbe58450035b7847
Reviewed-on: https://chromium-review.googlesource.com/776643
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517947}
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/fake_autocomplete_provider_client.cc
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/fake_autocomplete_provider_client.h
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/history_provider.cc
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/history_provider.h
[add] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/history_provider_unittest.cc
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/history_quick_provider_performance_unittest.cc
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/history_quick_provider_unittest.cc
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/history_url_provider_unittest.cc
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/in_memory_url_index.h
[modify] https://crrev.com/d62befdc3f11dafaed46f023829dab4fd79a59d0/components/omnibox/browser/shortcuts_provider_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30 2017

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

commit 58306c2d21ec4bf34cc9d30ba38890f6c6874193
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Nov 30 18:12:07 2017

[omnibox] Align tab icon vertically

Bottom wasn't lined up.

Bug: 780835
Change-Id: Id4f7dcc4d590e2f5f3f83457cea2095e7a1011cf
Reviewed-on: https://chromium-review.googlesource.com/800791
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520602}
[modify] https://crrev.com/58306c2d21ec4bf34cc9d30ba38890f6c6874193/components/omnibox/browser/vector_icons/tab.icon

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 1 2017

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

commit ea32dd9b3a39e0b7068318a7de59a06cdc37fda7
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Dec 01 03:34:38 2017

[omnibox] Make a couple tests use a common mock

Short-cuts provider and history URL provider unit-tests each have an
AnonFakeAutocompleteProviderClient, which shares a little code with
the common FakeAutocompleteProviderClient. This CL has those two
classes derive from the common one, and move the common bits down.
It's a follow-up to 776643.

Bug: 780835
Change-Id: Ida2fc850df0c36a35a3a1898296e85aec0a28809
Reviewed-on: https://chromium-review.googlesource.com/782079
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520839}
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/fake_autocomplete_provider_client.cc
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/fake_autocomplete_provider_client.h
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/history_provider_unittest.cc
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/history_quick_provider_performance_unittest.cc
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/history_quick_provider_unittest.cc
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/history_url_provider_unittest.cc
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/mock_autocomplete_provider_client.h
[modify] https://crrev.com/ea32dd9b3a39e0b7068318a7de59a06cdc37fda7/components/omnibox/browser/shortcuts_provider_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 12 2017

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

commit 17443734c217261df0dee9c4011d7ae4c6bf664b
Author: Kevin Bailey <krb@chromium.org>
Date: Tue Dec 12 22:07:57 2017

[omnibox] Move switch-to-tab handling into browser stack

The Omnibox was calling SwitchToTabWithURL() directly from OpenMatch()
for tab-switch suggestions. This change creates a special disposition
for the suggestions and adds support for it within the browser stack.

Bug: 780835
Change-Id: I4f48e6a7a4b46cf9c1a88baedafc87aaf763ebef
Reviewed-on: https://chromium-review.googlesource.com/809527
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523573}
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/browser_navigator_params.h
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.h
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/singleton_tabs.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/singleton_tabs.h
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/components/omnibox/browser/omnibox_edit_controller.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/components/omnibox/browser/omnibox_edit_controller.h
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/components/omnibox/browser/omnibox_view.cc
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/components/omnibox/browser/omnibox_view.h
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/ui/base/mojo/window_open_disposition.mojom
[modify] https://crrev.com/17443734c217261df0dee9c4011d7ae4c6bf664b/ui/base/window_open_disposition.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 5 2018

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

commit c8f9ef323c67d6207df5ec0383503653a41ee113
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Jan 05 21:00:21 2018

In browser navigation, do browser and tab search simultaneously

Previously, Browser::Navigate() first searched for a suitable browser,
then separately searched for a suitable tab within the browser, all
depending on the disposition.  Since a tab-searching disposition was
added, we now choose a browser depending on the tab within it.  Thus it
makes more sense to search for the browser and tab simultaneously,
saving that work.

Bug: 780835
Change-Id: I60848fb0e725547ed55962faad07b45376d4e2fd
Reviewed-on: https://chromium-review.googlesource.com/848745
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527379}
[modify] https://crrev.com/c8f9ef323c67d6207df5ec0383503653a41ee113/chrome/browser/ui/browser_list.cc
[modify] https://crrev.com/c8f9ef323c67d6207df5ec0383503653a41ee113/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/c8f9ef323c67d6207df5ec0383503653a41ee113/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/c8f9ef323c67d6207df5ec0383503653a41ee113/chrome/browser/ui/browser_navigator_params.h
[modify] https://crrev.com/c8f9ef323c67d6207df5ec0383503653a41ee113/chrome/browser/ui/singleton_tabs.cc
[modify] https://crrev.com/c8f9ef323c67d6207df5ec0383503653a41ee113/chrome/browser/ui/singleton_tabs.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 11 2018

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

commit 26d63890c1c2981a54450932cf2e0d34f026768b
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Jan 11 21:22:09 2018

[omnibox] Don't consider the active tab for switching

We currently suggest the current tab for switching to, even though we
block switching to it. This change prevents even suggesting the tab
(but will suggest another matching tab.)

Bug: 780835
Change-Id: If10e7991f584903b0ed57f39e2c1b44a43eeb4e3
Reviewed-on: https://chromium-review.googlesource.com/854773
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528756}
[modify] https://crrev.com/26d63890c1c2981a54450932cf2e0d34f026768b/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/26d63890c1c2981a54450932cf2e0d34f026768b/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/26d63890c1c2981a54450932cf2e0d34f026768b/chrome/browser/ui/singleton_tabs.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 1 2018

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

commit 0bcf0ae4bd73b3602f1945e859cfb9c519113806
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Mar 01 15:35:44 2018

[omnibox] Add button to tab switch suggestions

Feature change which adds a button to tab switch suggestions (under a
flag). Currently the flag does nothing but let the user evaluate it.
In a subsequent change, the button will invert the selection logic
such that <enter> will navigate, <shift-enter> will switch and the
button will switch.

Bug: 780835
Change-Id: I4453598f95db22fed8cc0bde50ef6d9788a205b2
Reviewed-on: https://chromium-review.googlesource.com/919247
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540163}
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/chrome/browser/about_flags.cc
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/chrome/browser/ui/views/omnibox/omnibox_result_view.h
[add] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[add] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/components/omnibox/browser/history_quick_provider.cc
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/components/omnibox/browser/history_url_provider.cc
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/components/omnibox/browser/omnibox_field_trial.h
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/components/omnibox/browser/omnibox_view.cc
[modify] https://crrev.com/0bcf0ae4bd73b3602f1945e859cfb9c519113806/tools/metrics/histograms/enums.xml

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 1 2018

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

commit 5ac5785a9c0b9921b24dd0b97ddbaed5491ba9f3
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Mar 01 23:19:27 2018

[omnibox] Fix tab switch suggestion visuals when button present

When we're using a button to trigger tab switches, we don't want the
description or icon hinting at switching, since the main suggestion
won't switch in this state.

In addition to these tiny changes, it adds a new icon for the button,
per the UI spec.

Bug: 780835
Change-Id: I01d0c913292852e6ecaf569c1feecccbebd0646d
Reviewed-on: https://chromium-review.googlesource.com/943831
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540338}
[modify] https://crrev.com/5ac5785a9c0b9921b24dd0b97ddbaed5491ba9f3/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h
[modify] https://crrev.com/5ac5785a9c0b9921b24dd0b97ddbaed5491ba9f3/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/5ac5785a9c0b9921b24dd0b97ddbaed5491ba9f3/components/omnibox/browser/autocomplete_match.cc
[modify] https://crrev.com/5ac5785a9c0b9921b24dd0b97ddbaed5491ba9f3/components/omnibox/browser/history_provider.cc
[add] https://crrev.com/5ac5785a9c0b9921b24dd0b97ddbaed5491ba9f3/components/omnibox/browser/vector_icons/switch.icon

Labels: Needs-Feedback
krb@ Tested this issue on Windows 10, Ubuntu 14.04 and Mac OS 10.12.6 on the latest Canary 66.0.3359.0.

On typing chrome in omnibox, can see the behavior as in the attached screen shot.
Request you to please check and confirm if anything is missed from our end in testing the issue.

Also request you to check the attached screen shot and help us in verifying the fix.

Thanks..
780835.png
12.9 KB View Download

Comment 17 by k...@chromium.org, Mar 2 2018

Susan, this is a new feature, not a bug. I can't tell what you're trying to do from your screenshot.
susan.boorgula: Thanks, but this feature is not ready for testing yet. We'll file a launch bug when we reach the point where we need to start getting test coverage.
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 5 2018

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

commit 34f55e76a77f7e166c4a45d2890e8af70fe54fb9
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Mar 05 23:43:49 2018

[omnibox, browser stack] Select tab before potentially closing other

If we close a tab, the tab index order can change, so, in
Navigate(), use the index before closing the other tab.

Bug: 780835
Change-Id: Ie88ab8e50528ec41e1048b3b5d12137f235202e3
Reviewed-on: https://chromium-review.googlesource.com/948822
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540987}
[modify] https://crrev.com/34f55e76a77f7e166c4a45d2890e8af70fe54fb9/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/34f55e76a77f7e166c4a45d2890e8af70fe54fb9/chrome/browser/ui/browser_navigator_browsertest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 8 2018

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

commit 3eb94aa5274b44133201abadda7ebbd30266e7a5
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Mar 08 17:22:48 2018

[omnibox] Initial MdTextButton support

Switch to use a base class of MdTextButton and make changes necessary
to support it.

Also includes some recent Omnibox coloring merges.

Bug: 780835
Change-Id: If3a9b007ff9ef82188f4ffa079ea8d8139d19daa
Reviewed-on: https://chromium-review.googlesource.com/949824
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541820}
[modify] https://crrev.com/3eb94aa5274b44133201abadda7ebbd30266e7a5/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/3eb94aa5274b44133201abadda7ebbd30266e7a5/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 12 2018

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

commit 55841fa7cf75a500ea75883b56e6cb3d49fa8980
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Mar 12 22:52:05 2018

[omnibox, browser stack] Make URL comparison looser for singletons

For dispositions of singleton and switch-to-tab, allow URLs which only
differ between http and https to match. This is done to allow
suggestions of http to find their ultimate destination.

Also happens to remove the enum NavigationPararms::IGNORE_AND_STAY_PUT
since it wasn't really being used, and enum NavigationParams::
RefBehavior since we force IGNORE_REF now.

Bug: 780835
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I73e7db51fdbef710b804966b482cb2cf0d6139ab
Reviewed-on: https://chromium-review.googlesource.com/949569
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542639}
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/autocomplete/chrome_autocomplete_provider_client.h
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/extensions/extension_tab_util.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/ui/browser_navigator_params.h
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/chrome/browser/ui/singleton_tabs.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/autocomplete_provider_client.h
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/fake_autocomplete_provider_client.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/fake_autocomplete_provider_client.h
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/history_provider.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/history_provider.h
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/history_provider_unittest.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/history_quick_provider.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/history_url_provider.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/components/omnibox/browser/mock_autocomplete_provider_client.h
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
[modify] https://crrev.com/55841fa7cf75a500ea75883b56e6cb3d49fa8980/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 13 2018

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

commit 53d4f974e8e181db2150d9c57565f0f965c09859
Author: Kevin Bailey <krb@chromium.org>
Date: Tue Mar 13 23:20:54 2018

[omnibox] Clean up for tab switch button event handling

Button and LabelButton have prescribed behaviors which one does not
need to re-implement when using them. One can instead use the
end-result event reporting, done through the method, StateChanged().
This CL leverages that method, and removes all its OnMouse methods.

This CL includes other minor clean-up, such as adding a SetHovered()
method, removal of unused methods and making methods private which
can be.

Bug: 780835
Change-Id: I1fa46ba22a4e02096bcacd79edf1c685fcee7ba5
Reviewed-on: https://chromium-review.googlesource.com/943599
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542946}
[modify] https://crrev.com/53d4f974e8e181db2150d9c57565f0f965c09859/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/53d4f974e8e181db2150d9c57565f0f965c09859/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Cc: nyerramilli@chromium.org glen@chromium.org ben@chromium.org anan...@chromium.org
 Issue 46623  has been merged into this issue.
Labels: -Needs-Feedback
Components: UI>Browser>Omnibox
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 23 2018

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

commit 6a221d2cf03258c1f91c38822c119b7ae8a76a43
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Mar 23 23:32:06 2018

Update metrics protobufs

Primarily adds 2 fields to the Omnibox event proto, but also makes
tiny changes to other protos to bring them up to date with the master
copies. About the 2 new fields:

The first indicates whether the autocomplete match also advertised
that a tab switch suggestion was present. i.e. whether the URL was
already open in another tab.

The second indicates whether the user selected the tab switch option
(or whether they chose to navigate).

Tab switch suggestion is orthogonal from the suggestion type because
most any suggestion can have its URL be present in another tab.
Additionally we wish to know the original type in order to give credit
to that provider.

We wish to know whether the user selected the tab switch option to
gauge whether the indication for the tab switch option was obvious
enough, and clear enough (if it was noticed).

Bug: 780835
Change-Id: Ib4e8af9275283fe0eac3c7734e1647a4b2c0a133
Reviewed-on: https://chromium-review.googlesource.com/977047
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545624}
[modify] https://crrev.com/6a221d2cf03258c1f91c38822c119b7ae8a76a43/third_party/metrics_proto/README.chromium
[modify] https://crrev.com/6a221d2cf03258c1f91c38822c119b7ae8a76a43/third_party/metrics_proto/omnibox_event.proto
[modify] https://crrev.com/6a221d2cf03258c1f91c38822c119b7ae8a76a43/third_party/metrics_proto/ukm/source.proto

Project Member

Comment 27 by bugdroid1@chromium.org, Mar 24 2018

Labels: M-68 Target-68
Adding Target-68 based on internal tracker. 
Project Member

Comment 29 by bugdroid1@chromium.org, Apr 4 2018

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

commit bd3b59530c68ecbac2d34d44060a6f68199470cd
Author: Kevin Bailey <krb@chromium.org>
Date: Wed Apr 04 03:50:06 2018

[omnibox] Move tab switch button handling to parent

It was suggested to make the parent OmniboxResultView the recipient
of button presses. This CL makes that change, and minor clean up to
privatize methods which no longer need to be public.

Bug: 780835
Change-Id: If124040f26a41839fad9518bac1be23c3f52cf88
Reviewed-on: https://chromium-review.googlesource.com/990788
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547971}
[modify] https://crrev.com/bd3b59530c68ecbac2d34d44060a6f68199470cd/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/bd3b59530c68ecbac2d34d44060a6f68199470cd/chrome/browser/ui/views/omnibox/omnibox_result_view.h
[modify] https://crrev.com/bd3b59530c68ecbac2d34d44060a6f68199470cd/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/bd3b59530c68ecbac2d34d44060a6f68199470cd/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 30 by bugdroid1@chromium.org, Apr 6 2018

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

commit e893d0e2a8c5711ffeba045decce0b4e0363c103
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Apr 06 01:28:10 2018

[omnibox] Add has_tab_switch fields to Omnibox metrics proto

Change from a strategy of having a separate suggestion type for tab switch suggestions
to preserving the original type and (instead) adding flags which indicate that a tab
switch could have occurred and (separately) did. We also populate these fields for
metrics now.

TBR'ing a rename of a previously approved change.

TBR=calamity@chromium.org

Bug: 780835
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I81a6e1c4173ae640e591cf53894c7dcc99ef2556
Reviewed-on: https://chromium-review.googlesource.com/963009
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548630}
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/chrome/browser/android/omnibox/autocomplete_controller_android.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/chrome/browser/ui/app_list/search/omnibox_result.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/chrome/browser/vr/ui_scene_creator.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/autocomplete_match.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/autocomplete_match.h
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/autocomplete_match_type.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/autocomplete_match_type.h
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/history_provider.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/history_provider.h
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/omnibox_log.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/omnibox_log.h
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/omnibox_metrics_provider.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/omnibox_popup_model.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/omnibox_view.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/components/omnibox/browser/shortcuts_backend.cc
[modify] https://crrev.com/e893d0e2a8c5711ffeba045decce0b4e0363c103/ios/chrome/browser/ui/omnibox/omnibox_util.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Apr 18 2018

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

commit ffc62220868cc5a07a92402edcb1c06c6250a2e9
Author: Kevin Bailey <krb@chromium.org>
Date: Wed Apr 18 18:58:14 2018

[omnibox] Try to convert all suggestions to tab switch suggestions

Currently we only attempt to convert history provider suggestions to
tab switch suggestions. This change moves the code to autocomplete
result and calls it from the controller after settling on the best
matches.

Also moves the unit test.

Bug: 780835
Change-Id: Ide8e7998911455766b8157d6dcbf8d65d32fd179
Reviewed-on: https://chromium-review.googlesource.com/1007482
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551765}
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/autocomplete_controller.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/autocomplete_result.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/autocomplete_result.h
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/autocomplete_result_unittest.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/fake_autocomplete_provider_client.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/fake_autocomplete_provider_client.h
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/history_provider.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/history_provider.h
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/history_provider_unittest.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/history_quick_provider.cc
[modify] https://crrev.com/ffc62220868cc5a07a92402edcb1c06c6250a2e9/components/omnibox/browser/history_url_provider.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Apr 20 2018

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

commit a6ab56bb578ed4b0eb615591c6206cf638531dcb
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Apr 20 17:50:57 2018

[omnibox] Remove references to tab switch button flag

We've all but decided to use the tab switch button, so only support
enabling tab switches or not. This CL only removes references to
that particular setting/value, not the setting itself.

Bug: 780835
Change-Id: I4d50cb998b71c95dcef8f9cf18d814501836010a
Reviewed-on: https://chromium-review.googlesource.com/1020401
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552377}
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/components/omnibox/browser/autocomplete_match.cc
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/components/omnibox/browser/autocomplete_result.cc
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/components/omnibox/browser/autocomplete_result_unittest.cc
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/components/omnibox/browser/omnibox_field_trial.h
[modify] https://crrev.com/a6ab56bb578ed4b0eb615591c6206cf638531dcb/components/omnibox/browser/omnibox_view.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Apr 27 2018

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

commit dbcdc350a0ea04a118ad320557f399f069175f3a
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Apr 27 13:57:14 2018

[omnibox] Handle tabbing to the tab switch button

Within the Omnibox suggestions pop-up list, the tab key will act like
a down arrow (and shift-tab, the up arrow), unless a suggestion is a
keyword suggestion, obviously. This change makes focus jump to the
tab switch button, if present, on tab key presses, before going on to
the next suggestion. Likewise, shift-tab is handled coming back up.
Finally, dragging also correctly changes the focus.

Bug: 780835
Change-Id: I3f33645ac36e0c03fa18267fb8bee9079b3cf297
Reviewed-on: https://chromium-review.googlesource.com/1014946
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554375}
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/components/omnibox/browser/omnibox_popup_model.cc
[modify] https://crrev.com/dbcdc350a0ea04a118ad320557f399f069175f3a/components/omnibox/browser/omnibox_popup_model.h

Project Member

Comment 34 by bugdroid1@chromium.org, Apr 28 2018

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

commit c211988810bca38394c8a72d6c35abaaca45c14b
Author: Kevin Bailey <krb@chromium.org>
Date: Sat Apr 28 00:52:59 2018

[omnibox] Remove button option from tab switch suggestions setting

The code has already been modified to assume this option if enabled.
This CL actually removes the option.

Bug: 780835
Change-Id: I6bc84f7eb7763136d1effc40659f65297396b7b8
Reviewed-on: https://chromium-review.googlesource.com/1033574
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554586}
[modify] https://crrev.com/c211988810bca38394c8a72d6c35abaaca45c14b/chrome/browser/about_flags.cc
[modify] https://crrev.com/c211988810bca38394c8a72d6c35abaaca45c14b/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/c211988810bca38394c8a72d6c35abaaca45c14b/components/omnibox/browser/omnibox_field_trial.h

Project Member

Comment 35 by bugdroid1@chromium.org, May 3 2018

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

commit cd966170392dcf05f62d77ec17e5f371c7a2d83a
Author: Kevin Bailey <krb@chromium.org>
Date: Thu May 03 16:20:59 2018

[omnibox] Make GURLToStrippedGURL() remove refs

Refs (or hashes) are useful for navigating, but not useful for
comparing URLs. We compare URLs when deduping URLs in Omnibox
matches, and when testing eligibility for switching to already
open tabs. This CL changes GURLToStrippedGURL() to remove refs
from the stripped version of destination URLs (only used for
comparison) and when testing URLs for tab switching.

We only remove refs when the input doesn't specify a ref.
This is critical to proper deduping.

Also added unit test.

Bug: 780835
Change-Id: Ifef8d8b9d2efd4394b598342c13ef8aa439089ce
Reviewed-on: https://chromium-review.googlesource.com/1025953
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555760}
[modify] https://crrev.com/cd966170392dcf05f62d77ec17e5f371c7a2d83a/chrome/browser/autocomplete/chrome_autocomplete_provider_client_unittest.cc
[modify] https://crrev.com/cd966170392dcf05f62d77ec17e5f371c7a2d83a/components/omnibox/browser/autocomplete_input.cc
[modify] https://crrev.com/cd966170392dcf05f62d77ec17e5f371c7a2d83a/components/omnibox/browser/autocomplete_input.h
[modify] https://crrev.com/cd966170392dcf05f62d77ec17e5f371c7a2d83a/components/omnibox/browser/autocomplete_match.cc

Project Member

Comment 36 by bugdroid1@chromium.org, May 10 2018

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

commit 13622d510f53ddbd7633727c6834a905d6105eef
Author: Kevin Bailey <krb@chromium.org>
Date: Thu May 10 16:25:04 2018

[omnibox] Leverage the existing file URL parsing routine

A new file URL parsing routine was added in a previous CL (1025953)
for detecting hash/pound refs in file URLs. This made file: schemes
a bit of an exception. This CL removes that routine and leverages
the existing routine, calling it from where the other URL types
are parsed, treating file: schemes like the other schemes.

Bug: 780835
Change-Id: Ic7f00b88e50801d00c609c1a92d00d1f1bf67808
Reviewed-on: https://chromium-review.googlesource.com/1043157
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557547}
[modify] https://crrev.com/13622d510f53ddbd7633727c6834a905d6105eef/components/certificate_transparency/ct_policy_manager_unittest.cc
[modify] https://crrev.com/13622d510f53ddbd7633727c6834a905d6105eef/components/omnibox/browser/autocomplete_input.cc
[modify] https://crrev.com/13622d510f53ddbd7633727c6834a905d6105eef/components/omnibox/browser/autocomplete_input.h
[modify] https://crrev.com/13622d510f53ddbd7633727c6834a905d6105eef/components/url_formatter/url_fixer.cc
[modify] https://crrev.com/13622d510f53ddbd7633727c6834a905d6105eef/components/url_formatter/url_fixer_unittest.cc

Project Member

Comment 37 by bugdroid1@chromium.org, May 14 2018

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

commit 367fc70d8c50050cee7de63aea8f650c87a2f9b0
Author: Kevin Bailey <krb@chromium.org>
Date: Mon May 14 20:15:24 2018

[omnibox] Tweak tab switch button text

Update the text of the tab switch button to latest UI recommendation,
and also noticed we weren't using the resource constant.

Bug: 780835
Change-Id: I43e685d7fff4ee7c62812a1a6a22a2914cf9b359
Reviewed-on: https://chromium-review.googlesource.com/1057698
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558434}
[modify] https://crrev.com/367fc70d8c50050cee7de63aea8f650c87a2f9b0/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/367fc70d8c50050cee7de63aea8f650c87a2f9b0/components/omnibox_strings.grdp

Project Member

Comment 38 by bugdroid1@chromium.org, May 16 2018

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

commit ac0b311998440fdc553ce505f07acd74e3fd6a28
Author: Kevin Bailey <krb@chromium.org>
Date: Wed May 16 14:55:34 2018

[omnibox] Move padding into tab switch button class

We were adding padding both in the call to, and within the Omnibox tab
switch button class. This CL moves all the padding to the class, which
removes the need for the constant in the parent class, Omnibox result
view.

Bug: 780835
Change-Id: I1c5255752625b08904c356369989073063a3e904
Reviewed-on: https://chromium-review.googlesource.com/1059706
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559090}
[modify] https://crrev.com/ac0b311998440fdc553ce505f07acd74e3fd6a28/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/ac0b311998440fdc553ce505f07acd74e3fd6a28/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/ac0b311998440fdc553ce505f07acd74e3fd6a28/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 39 by bugdroid1@chromium.org, May 17 2018

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

commit 644b52c4a709f91b413a4886c9adca83bdb57ba3
Author: Kevin Bailey <krb@chromium.org>
Date: Thu May 17 21:21:20 2018

[omnibox] Narrow tab switch button when view narrows

Narrow tab switch button, in increments, per spec, as the parent
window narrows. The button will lose text as it narrows,
eventually disappearing. The procession is (full text), just
'switch', no text (just the icon), and finally disappearing.

A future CL will cause the button to animate to each size.
The relationship between widths (the function to calculate
which width the button should choose) will probably change
as well.

Bug: 780835
Change-Id: Ic6f6183f1dc1a6090bf421c64dd8cb0b24fd8e90
Reviewed-on: https://chromium-review.googlesource.com/1019881
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559681}
[modify] https://crrev.com/644b52c4a709f91b413a4886c9adca83bdb57ba3/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/644b52c4a709f91b413a4886c9adca83bdb57ba3/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/644b52c4a709f91b413a4886c9adca83bdb57ba3/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 40 by bugdroid1@chromium.org, May 22 2018

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

commit 72d34b6f227c892eed70bc859d2f2a9cc9145274
Author: Kevin Bailey <krb@chromium.org>
Date: Tue May 22 19:17:26 2018

[omnibox] Have result view pass its index to OpenMatch

We were calling the version of OpenMatch() that did not take an index,
which opened the result view that was "selected" (highlighted). But
with the tab switch button present, the user can click on a result
that is not the selected result. By passing the view's index, this
opens the specific URL that the user chose explicitly.

Bug: 780835
Change-Id: Ie60e28a2106f3ad581aa640dad7ee5d134dee4ed
Reviewed-on: https://chromium-review.googlesource.com/1069305
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560722}
[modify] https://crrev.com/72d34b6f227c892eed70bc859d2f2a9cc9145274/chrome/browser/ui/views/omnibox/omnibox_result_view.cc

Project Member

Comment 41 by bugdroid1@chromium.org, May 22 2018

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

commit 6a43dad748e0d4f51eb6af5a06610dcca0d88b0b
Author: Kevin Bailey <krb@chromium.org>
Date: Tue May 22 21:00:25 2018

[omnibox] Add another official string for tab switch button

Add the other official string - the shortened version of the
wider button label - and have the tab switch button use it.

Bug: 780835
Change-Id: I51aed2d19a9abd15b613e29becb929b1aa483a07
Reviewed-on: https://chromium-review.googlesource.com/1069255
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560770}
[modify] https://crrev.com/6a43dad748e0d4f51eb6af5a06610dcca0d88b0b/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/6a43dad748e0d4f51eb6af5a06610dcca0d88b0b/components/omnibox_strings.grdp

Comment 42 by k...@chromium.org, May 23 2018

Latest slide animation of button changing size, with duration extended from .5 to 2 seconds:

https://screencast.googleplex.com/cast/NjMxODU4ODg2MDMwMTMxMnxkYmNhNzJiMC0zMg

Project Member

Comment 43 by bugdroid1@chromium.org, May 24 2018

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

commit 6895083eca6d3d7b0564ab367755967bc5664104
Author: Kevin Bailey <krb@chromium.org>
Date: Thu May 24 16:01:04 2018

[omnibox] Change multi-value tab switch flag to binary feature

Fundamentally, change MULTI_VALUE_TYPE to FEATURE_VALUE_TYPE for the
tab switch suggestions flag, since we don't need the 'button' option
any more. Should be functionally equivalent.

Bug: 780835
Change-Id: I56f2d20a577b0755935a5d3de6e9ae6f5c6ba8d9
Reviewed-on: https://chromium-review.googlesource.com/1069805
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561514}
[modify] https://crrev.com/6895083eca6d3d7b0564ab367755967bc5664104/chrome/browser/about_flags.cc
[modify] https://crrev.com/6895083eca6d3d7b0564ab367755967bc5664104/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/6895083eca6d3d7b0564ab367755967bc5664104/components/omnibox/browser/omnibox_field_trial.h

Project Member

Comment 44 by bugdroid1@chromium.org, May 25 2018

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

commit 90d23f33cbb2391eb62dc839f6acdca854860160
Author: Kevin Bailey <krb@chromium.org>
Date: Fri May 25 21:08:50 2018

[omnibox] Change size of tab switch button and icon

The tab switch button wasn't quite vertically centered in the GM2
style result view, and the icon was being clipped. This CL makes
provisional size changes to make them fit.

Bug: 780835
Change-Id: Ia72cb406088f50c2e22c0929e9e0acdb8d0db31e
Reviewed-on: https://chromium-review.googlesource.com/1069569
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562010}
[modify] https://crrev.com/90d23f33cbb2391eb62dc839f6acdca854860160/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/90d23f33cbb2391eb62dc839f6acdca854860160/components/omnibox/browser/vector_icons/switch.icon

Project Member

Comment 45 by bugdroid1@chromium.org, May 30 2018

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

commit 6f95417ff396446ba6c14466bf1d566a6bc7d44c
Author: Kevin Bailey <krb@chromium.org>
Date: Wed May 30 18:40:38 2018

[omnibox] Animate tab switch button size changes

The tab switch button only exists at quantized sizes, depending on
window width. Currently it jumps between those sizes. We would prefer
that it animate between them. This CL adds the beginning of animation,
putting off issues like delaying animation until window shrinking
stops, etc.

The CL also changes the ellision type to fade.

Bug: 780835
Change-Id: Ic4a9997e213bae5415d73a7f83b410b788a95279
Reviewed-on: https://chromium-review.googlesource.com/1067667
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562928}
[modify] https://crrev.com/6f95417ff396446ba6c14466bf1d566a6bc7d44c/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/6f95417ff396446ba6c14466bf1d566a6bc7d44c/chrome/browser/ui/views/omnibox/omnibox_result_view.h
[modify] https://crrev.com/6f95417ff396446ba6c14466bf1d566a6bc7d44c/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/6f95417ff396446ba6c14466bf1d566a6bc7d44c/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 46 by bugdroid1@chromium.org, Jun 7 2018

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

commit bca3560a4b641a1a3c03ed6c124aa5fc4e930066
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Jun 07 15:05:50 2018

[omnibox] Correctly arrange tab switch button methods

Bug: 780835
Change-Id: I89b56680b3ffea40c8a87a42423d182268c06615
Reviewed-on: https://chromium-review.googlesource.com/1082491
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565266}
[modify] https://crrev.com/bca3560a4b641a1a3c03ed6c124aa5fc4e930066/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc

Blockedon: 851015
Project Member

Comment 48 by bugdroid1@chromium.org, Jun 14 2018

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

commit 44122289d9cf914eb5afa62547ce5574914090b5
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Jun 14 15:33:35 2018

[omnibox] Use two line format for tab switch suggestions

A result looks crowded with a tab switch button. The spec asks that
we use the two line format if the tab switch button is present.

This CL also expands the button to 32px, per spec.

Bug: 780835
Change-Id: I372cb24646bdd448497f0bcf456d383a04a8a55b
Reviewed-on: https://chromium-review.googlesource.com/1087754
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567280}
[modify] https://crrev.com/44122289d9cf914eb5afa62547ce5574914090b5/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc
[modify] https://crrev.com/44122289d9cf914eb5afa62547ce5574914090b5/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.h
[modify] https://crrev.com/44122289d9cf914eb5afa62547ce5574914090b5/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
[modify] https://crrev.com/44122289d9cf914eb5afa62547ce5574914090b5/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/44122289d9cf914eb5afa62547ce5574914090b5/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 49 by bugdroid1@chromium.org, Jun 15 2018

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

commit a808d9f322f9d81b886644106b87294ff7ed24d2
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Jun 15 16:54:01 2018

[omnibox] Improve dimensions and icon of tab switch button

This CL makes the dimensions within the tab switch button a closer
to the spec: 8px between icon and text, and reduces the 18px to the
left of the icon to 16px. (The spec still wants 8px here.)

Additionally, it updates the icon using the provided SVG.

Bug: 780835
Change-Id: I4542ea03c4097b83c959f77ed86c96b2744e14ce
Reviewed-on: https://chromium-review.googlesource.com/1101399
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567693}
[modify] https://crrev.com/a808d9f322f9d81b886644106b87294ff7ed24d2/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/a808d9f322f9d81b886644106b87294ff7ed24d2/components/omnibox/browser/vector_icons/switch.icon

Project Member

Comment 50 by bugdroid1@chromium.org, Jun 18 2018

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

commit 1db45fd06a86d0e5b040d571b252cb2b1e7471ee
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Jun 18 15:27:52 2018

[omnibox] Fix two-line view inset and image choice

We missed a place where an inset needed to be tweaked for tab switch
suggestions. Also, it was choosing the wrong image for rich + tab
switch suggestion.

Bug: 780835
Change-Id: I2f74618584480d9857a5e5c5e7c82636735d7c6b
Reviewed-on: https://chromium-review.googlesource.com/1102913
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568015}
[modify] https://crrev.com/1db45fd06a86d0e5b040d571b252cb2b1e7471ee/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc

Project Member

Comment 51 by bugdroid1@chromium.org, Jun 18 2018

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

commit a65fc33c0b027a6450b7bfc9a5cd5c26e6d641d5
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Jun 18 19:08:47 2018

[omnibox] Correct tab switch button icon color

...to standard Chrome icon color.

Bug: 780835
Change-Id: Ie377265d7f50a8598075aca9b7868239750b2c6f
Reviewed-on: https://chromium-review.googlesource.com/1104787
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568098}
[modify] https://crrev.com/a65fc33c0b027a6450b7bfc9a5cd5c26e6d641d5/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc

Project Member

Comment 52 by bugdroid1@chromium.org, Jun 19 2018

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

commit caa76e718de62ddf8f93b2f2aa3bc1c211af9a60
Author: Kevin Bailey <krb@chromium.org>
Date: Tue Jun 19 16:17:06 2018

[omnibox] Fix tab switch button pressed coloring

Restores (dark) pressed color to tab switch button, which was
unintentionally removed in prior CL. Also creates an ink
drop mask for the tab switch button, due to discovering that ink drop
was extending beyond border. (A more general solution is being
worked on.)

Bug: 780835
Change-Id: Id96f4c1695b0f0dceed1e122b55f0f029f813fa0
Reviewed-on: https://chromium-review.googlesource.com/1103140
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568479}
[modify] https://crrev.com/caa76e718de62ddf8f93b2f2aa3bc1c211af9a60/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
[modify] https://crrev.com/caa76e718de62ddf8f93b2f2aa3bc1c211af9a60/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h

Project Member

Comment 53 by bugdroid1@chromium.org, Jun 19 2018

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

commit d1d8274829c9686c2f6dc525f0d9114f7f48ad74
Author: Kevin Bailey <krb@chromium.org>
Date: Tue Jun 19 19:53:13 2018

[omnibox] Don't force 'pressed' color on tab switch button when focused

We used to highlight the tab switch button when focused. The spec now
asks for a focus ring instead of highlighting. This change allows the
background to return to following the mouse state.

Bug: 780835
Change-Id: Ie2a3c4b7bbe4c357855f19956f880c5cef01da21
Reviewed-on: https://chromium-review.googlesource.com/1106282
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568578}
[modify] https://crrev.com/d1d8274829c9686c2f6dc525f0d9114f7f48ad74/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc

Hi krb@, I noticed two small edge cases with the Tab Switch Button in latest Canary on macOS:

1.) When you hover over the button, the highlight overlaps the button and squares of the highlight appear. (Please see the first screenshot.)

2.) If only the Tab Switch Icon appears on button, then the icon is not centered on the button. (Please see the second and third screenshots.)

If you want, I can open separate reports for this.

Thanks
Mehmet

Tab_Switch_Button_Mouse_Hover.png
18.7 KB View Download
Actual_Tab_Switch_Button_icon_NOT_centered.png
53.4 KB View Download
Expected_Tab_Switch_Button_icon_centered.png
54.2 KB View Download

Comment 55 by k...@chromium.org, Jun 20 2018

> 1.) When you hover over the button, the highlight overlaps the button and squares of the highlight appear. (Please see the first screenshot.)

I've mentioned this to the MdTextButton authors. It seems to be a known issue, but not fixed yet. I don't know if it warrants a separate bug.

> 2.) If only the Tab Switch Icon appears on button, then the icon is not centered on the button. (Please see the second and third screenshots.)

hmm, that shouldn't be. Might be a recent change. It definitely used to work. Sounds bug-worthy.
> I've mentioned this to the MdTextButton authors. It seems to be a known issue, but not fixed yet. I don't know if it warrants a separate bug.

Okay, thanks. Let's wait then... It will probably fixed then in near future :)

> hmm, that shouldn't be. Might be a recent change. It definitely used to work. Sounds bug-worthy.

I did a bisect and this is the regression range:  https://chromium.googlesource.com/chromium/src/+log/6a9b06a57b33a5a955af04a914f85f981cf05f88..a808d9f322f9d81b886644106b87294ff7ed24d2

Probably caused by https://chromium.googlesource.com/chromium/src/+/a808d9f322f9d81b886644106b87294ff7ed24d2

Comment 57 by k...@chromium.org, Jun 20 2018

Sorry, didn't realize you were still looking at this. I already guessed and verified that it was the align-left (although that should actually work, another library issue).

I'll save you the trouble of a bug, and just check-in against this one.
> Sorry, didn't realize you were still looking at this. I already guessed and verified that it was the align-left (although that should actually work, another library issue).

I'll save you the trouble of a bug, and just check-in against this one.


No problem and thanks :)
Project Member

Comment 59 by bugdroid1@chromium.org, Jun 22 2018

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

commit 6a3603c33498790e780074e1f9cfb59745bf0fa3
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Jun 22 17:12:45 2018

[omnibox] Set dimensions of tab switch button before measuring

We were measuring the tab switch button size before setting all the
relevant aspects. When we requested a size, it was therefore too big.
This corrects that and, since the size is more appropriate, we don't
need the left justification, which was broken when there wasn't any
text anyways.

Bug: 780835
Change-Id: I72bd6aedc84837864dc6fe764e4ce865f9170cb4
Reviewed-on: https://chromium-review.googlesource.com/1108452
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569670}
[modify] https://crrev.com/6a3603c33498790e780074e1f9cfb59745bf0fa3/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc

Project Member

Comment 60 by bugdroid1@chromium.org, Jun 29 2018

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

commit 367d5b86867a4fcecd37992cc0669ba2cc22c260
Author: Kevin Bailey <krb@chromium.org>
Date: Fri Jun 29 22:27:00 2018

[omnibox] Preserve focus of tab switch button if nothing changes

If the slightest thing changes in the Omnibox, even if the results
don't actually change, we clear the focused state of the tab switch
button. This change makes it so that, if the selected match has
a focused tab switch button, and the match doesn't change, preserve
the focus state.

Bug: 780835
Change-Id: Ib9a1f0546679c71d7097ba9ecde3d3374967c051
Reviewed-on: https://chromium-review.googlesource.com/1117621
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571679}
[modify] https://crrev.com/367d5b86867a4fcecd37992cc0669ba2cc22c260/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/367d5b86867a4fcecd37992cc0669ba2cc22c260/components/omnibox/browser/autocomplete_controller.h
[modify] https://crrev.com/367d5b86867a4fcecd37992cc0669ba2cc22c260/components/omnibox/browser/omnibox_popup_model.cc
[modify] https://crrev.com/367d5b86867a4fcecd37992cc0669ba2cc22c260/components/omnibox/browser/omnibox_popup_model.h
[modify] https://crrev.com/367d5b86867a4fcecd37992cc0669ba2cc22c260/components/omnibox/browser/omnibox_popup_model_unittest.cc

Project Member

Comment 61 by bugdroid1@chromium.org, Jul 16

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

commit 1497a3e0cdf872b7ab7257e1d941da466c9b4eb5
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Jul 16 15:53:15 2018

[omnibox] Add accessibility label to suggestions with tab switch button

For suggestions with a tab switch button, advertise the ability to tab
over to it and press enter, in the accessibility label.

Bug: 780835,  853911 , 853929
Change-Id: I91649d0517ab6e75970e49b5358e7d210c914071
Reviewed-on: https://chromium-review.googlesource.com/1106706
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575283}
[modify] https://crrev.com/1497a3e0cdf872b7ab7257e1d941da466c9b4eb5/components/omnibox/browser/autocomplete_match_type.cc
[modify] https://crrev.com/1497a3e0cdf872b7ab7257e1d941da466c9b4eb5/components/omnibox_strings.grdp

Project Member

Comment 62 by bugdroid1@chromium.org, Jul 18

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

commit 958453892e08b5b077c8fe8beb1ec4da107dcd14
Author: Kevin Bailey <krb@chromium.org>
Date: Wed Jul 18 17:35:22 2018

[omnibox] Add tab switch button tooltip (and thus accessibility label)

Bug: 780835, 853929
Change-Id: I9f5018fa6522d76ab7a8c57b635b084764f482db
Reviewed-on: https://chromium-review.googlesource.com/1142129
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576120}
[modify] https://crrev.com/958453892e08b5b077c8fe8beb1ec4da107dcd14/chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc

Project Member

Comment 63 by bugdroid1@chromium.org, Aug 6

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

commit 7c1f759d4b95d998b0f0b078784172912e282f6c
Author: Kevin Bailey <krb@chromium.org>
Date: Mon Aug 06 15:18:25 2018

[omnibox] Have spacebar activate tab switch button

Spacebar normally inserts a space into the Omnibox. If the cursor (or
selection) is at the end of the text and the tab switch button is
"focused", instead activate the button.

Bug: 780835
Change-Id: I121f72bf3a979213d0506da1168a7658be84e706
Reviewed-on: https://chromium-review.googlesource.com/1160597
Reviewed-by: Justin Donnelly (OOO until Aug 13) <jdonnelly@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580873}
[modify] https://crrev.com/7c1f759d4b95d998b0f0b078784172912e282f6c/chrome/browser/ui/views/omnibox/omnibox_view_views.cc

Labels: Hotlist-ConOps-CrOS
Project Member

Comment 65 by bugdroid1@chromium.org, Aug 7

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

commit 14de10b2d466d91454e42284b9c4fd49d6d4a235
Author: Kevin Bailey <krb@chromium.org>
Date: Tue Aug 07 03:30:47 2018

[omnibox] Restrict tab switch button space activation to no modifiers

Currently one can activate the tab switch button with shift-space etc.
This CL restricts it to only space, no modifiers.

Bug: 780835
Change-Id: I49b50d9d693e6201d7c3e74c91929bf71280865f
Reviewed-on: https://chromium-review.googlesource.com/1163991
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581122}
[modify] https://crrev.com/14de10b2d466d91454e42284b9c4fd49d6d4a235/chrome/browser/ui/views/omnibox/omnibox_view_views.cc

Cc: viswa.karala@chromium.org k...@chromium.org
 Issue 878003  has been merged into this issue.

Sign in to add a comment