New issue
Advanced search Search tips

Issue 874592 link

Starred by 5 users

Issue metadata

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



Sign in to add a comment

Implement Query in Omnibox for Desktop

Project Member Reported by tommycli@chromium.org, Aug 15

Issue description

Implement Query in Omnibox for Desktop
 
Labels: -Type-Bug Type-Feature
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16

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

commit d6570450254cf07fb01ff6f3b015c48d9b3b2cad
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Aug 16 17:52:31 2018

Omnibox: Move Query in Omnibox feature flag to Desktop+Android

Query in Omnibox is currently only implemented in Android.

This CL moves the feature flag from Android-only to Android+Desktop in
preparation for implementing the feature in Desktop as well.

Bug: 874592
Change-Id: I7d24289d4fb07467061ae616610d9c58296e73ce
Reviewed-on: https://chromium-review.googlesource.com/1176432
Reviewed-by: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583723}
[modify] https://crrev.com/d6570450254cf07fb01ff6f3b015c48d9b3b2cad/chrome/browser/about_flags.cc
[modify] https://crrev.com/d6570450254cf07fb01ff6f3b015c48d9b3b2cad/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/d6570450254cf07fb01ff6f3b015c48d9b3b2cad/chrome/browser/android/chrome_feature_list.h
[modify] https://crrev.com/d6570450254cf07fb01ff6f3b015c48d9b3b2cad/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/d6570450254cf07fb01ff6f3b015c48d9b3b2cad/components/omnibox/browser/omnibox_field_trial.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 29

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

commit 44ae7f80fe9863448007d53bd08dcad2c01fc572
Author: Tommy C. Li <tommycli@chromium.org>
Date: Wed Aug 29 17:07:11 2018

Omnibox: Add Query In Omnibox native implementation.

This is a pretty-much direct port of the existing logic within
ToolbarModel.java for Query In Omnibox.

It's meant to eventually be used cross-platform on both Desktop as well
as on Android.

For now this CL just adds the native implementation with a test.

Bug: 874592
Change-Id: I27da02126d52d963b2e4c56d10b9cd1c16b0c8b5
Reviewed-on: https://chromium-review.googlesource.com/1188598
Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Reviewed-by: Troy Hildebrandt <thildebr@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587165}
[modify] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/DEPS
[add] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/query_in_omnibox.cc
[add] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/query_in_omnibox.h
[add] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/query_in_omnibox_unittest.cc
[modify] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/test_omnibox_client.cc
[modify] https://crrev.com/44ae7f80fe9863448007d53bd08dcad2c01fc572/components/omnibox/browser/test_omnibox_client.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30

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

commit 08acb8994f55b1c1ff31ef5820a86ba18d25ec09
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Aug 30 22:23:06 2018

Omnibox: Add Query in Omnibox Android to Native JNI bridge code

This adds the bridge code to allow Android Chrome to access the native
Query in Omnibox implementation that lives within the omnibox/
component.

Bug: 874592
Change-Id: I6d6721c648b9de453f4a17791bc91001ac104b1a
Reviewed-on: https://chromium-review.googlesource.com/1196162
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587810}
[add] https://crrev.com/08acb8994f55b1c1ff31ef5820a86ba18d25ec09/chrome/android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java
[modify] https://crrev.com/08acb8994f55b1c1ff31ef5820a86ba18d25ec09/chrome/android/java_sources.gni
[modify] https://crrev.com/08acb8994f55b1c1ff31ef5820a86ba18d25ec09/chrome/browser/BUILD.gn
[add] https://crrev.com/08acb8994f55b1c1ff31ef5820a86ba18d25ec09/chrome/browser/android/omnibox/query_in_omnibox_android.cc
[add] https://crrev.com/08acb8994f55b1c1ff31ef5820a86ba18d25ec09/chrome/browser/android/omnibox/query_in_omnibox_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 5

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

commit d5db3d9b1b63b30fbeb16706b86513b1f2859937
Author: Tommy C. Li <tommycli@chromium.org>
Date: Tue Sep 04 17:27:17 2018

Omnibox: Make QueryInOmniboxFactory so Java bridge is all-static.

QueryInOmnibox functionality is intrinsically profile-keyed, since it
relies on two other profile-keyed services (AutocompleteClassifier and
TemplateURLService).

This suggests that QueryInOmnibox should also be a
BrowserContextKeyedService, just like those two classes.

If QueryInOmnibox is profile-keyed, it would be burdensome to have the
Java side manage its own instances, so this CL also makes all the Java
exposed methods static, with a Profile key.

Bug: 874592
Change-Id: Ic8de22b4d19479a349129e24b9e8732a06002ff0
Reviewed-on: https://chromium-review.googlesource.com/1199818
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588567}
[modify] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/chrome/android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java
[modify] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/chrome/browser/BUILD.gn
[modify] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/chrome/browser/android/omnibox/query_in_omnibox_android.cc
[delete] https://crrev.com/9b3146ac472fdeab43fdc04d1d9adbb8ab9523c5/chrome/browser/android/omnibox/query_in_omnibox_android.h
[modify] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/chrome/browser/ui/omnibox/query_in_omnibox_factory.cc
[add] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/chrome/browser/ui/omnibox/query_in_omnibox_factory.h
[modify] https://crrev.com/d5db3d9b1b63b30fbeb16706b86513b1f2859937/components/omnibox/browser/query_in_omnibox.h

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 5

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

commit 7f3891971a8bed477dfd4c0b42c372a5ed618027
Author: Tommy C. Li <tommycli@chromium.org>
Date: Wed Sep 05 19:58:49 2018

Omnibox: Make QueryInOmnibox on Android use native implementation

This CL removes the Java implementation of Query in Omnibox, and makes
Chrome for Android use the native C++ implementation.

This native C++ implementation will be shared with Desktop.

Bug: 874592
Change-Id: Idfe296090c43d5d3aefe4ce166a4f8859d5699f1
Reviewed-on: https://chromium-review.googlesource.com/1204413
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588976}
[modify] https://crrev.com/7f3891971a8bed477dfd4c0b42c372a5ed618027/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/7f3891971a8bed477dfd4c0b42c372a5ed618027/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModel.java

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 6

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

commit c73ed2e3b1a1676a6df56b8ffdf20c901bac670a
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Sep 06 16:36:43 2018

Omnibox: Add QueryInOmnibox instance access to OmniboxClient

Desktop Omnibox component will need access to the profile-keyed
QueryInOmnibox service.

This can is provided by the embedder, so add its access to
OmniboxClient like the other embedder-provided profile-keyed services.

Bug: 874592
Change-Id: Ibc12b563a9d2fe7a2625cdc5fdc0d10bf85c45d7
Reviewed-on: https://chromium-review.googlesource.com/1208455
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589187}
[modify] https://crrev.com/c73ed2e3b1a1676a6df56b8ffdf20c901bac670a/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/c73ed2e3b1a1676a6df56b8ffdf20c901bac670a/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
[modify] https://crrev.com/c73ed2e3b1a1676a6df56b8ffdf20c901bac670a/chrome/browser/ui/omnibox/chrome_omnibox_client.h
[modify] https://crrev.com/c73ed2e3b1a1676a6df56b8ffdf20c901bac670a/components/omnibox/browser/omnibox_client.cc
[modify] https://crrev.com/c73ed2e3b1a1676a6df56b8ffdf20c901bac670a/components/omnibox/browser/omnibox_client.h

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 17

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

commit 4566bc073f2c5a9317c50785aeda61ed84325053
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Sep 17 22:08:10 2018

Omnibox: Query in Omnibox - Basic implementation for Views

This CL enables the very basic Query in Omnibox functionality in the
Views Omnibox (on Desktop).

Further refinements will be needed to disable "unelision" when Query
in Omnibox is enabled, as well as to show the correct icon, but this
is a good start.

TBR=ellyjones@chromium.org

Bug: 874592
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ic02228e18ac536bf9facfa7aeac95aee0152890f
Reviewed-on: https://chromium-review.googlesource.com/1220694
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591838}
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/chrome/browser/autocomplete/autocomplete_browsertest.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/query_in_omnibox.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/query_in_omnibox.h
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/test_omnibox_client.cc
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/components/omnibox/browser/test_omnibox_client.h
[modify] https://crrev.com/4566bc073f2c5a9317c50785aeda61ed84325053/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 18

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

commit 474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e
Author: Tommy C. Li <tommycli@chromium.org>
Date: Tue Sep 18 18:51:18 2018

Omnibox: Query in Omnibox - Show icon, and don't unelide search terms

This CL continues implementation of the Query in Omnibox feature for
Desktop with two main changes:

 1. Shows the Default Search Provider favicon in the Omnibox when we
    are displaying search terms.

 2. Don't (erroneously) try to do unelision for Steady State Elisions
    when we are displaying search terms. Adds a test for that.

This also adds a convenience method to OmniboxEditModel to prevent
repeated code, and adds a bit of testing code for the convenience
method.

Bug: 874592
Change-Id: Iafd791db349df434f616697ecbe6122285b5351b
Reviewed-on: https://chromium-review.googlesource.com/1229493
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592122}
[modify] https://crrev.com/474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc
[modify] https://crrev.com/474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/474bc47ca7d6ccc9e2e1775b72339e9c4d68a08e/components/omnibox/browser/omnibox_view.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 4

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

commit 73d1f57e332620780afa57aa7615b66026fd9bcc
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Oct 04 19:43:38 2018

Omnibox: Query in Omnibox - Fix Omnibox::CurrentTextIsURL() logic.

This logic needed a bit of updating to accomodate the Query in Omnibox
case. I haven't seen any concrete instances of user-visible breakages,
but I did notice that this logic is wrong.

Bug: 874592
Change-Id: Ie7ec2ec71e463ccd919d42edad06d3ffd35e5313
Reviewed-on: https://chromium-review.googlesource.com/c/1260006
Reviewed-by: Kevin Bailey <krb@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596811}
[modify] https://crrev.com/73d1f57e332620780afa57aa7615b66026fd9bcc/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/73d1f57e332620780afa57aa7615b66026fd9bcc/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/73d1f57e332620780afa57aa7615b66026fd9bcc/components/omnibox/browser/omnibox_edit_model_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 5

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

commit c6dfa55a3a5c70313bcd5c5cb736dd40d33f03bc
Author: Tommy C. Li <tommycli@chromium.org>
Date: Fri Oct 05 18:07:05 2018

Omnibox: Query in Omnibox - Add a general unelide method on model

This CL takes a section of code used to show the full URL for Steady
State Elisions and moves it from the Views UI code to the
OmniboxEditModel.

This is in preparation for it to be used for Query in Omnibox. It also
adds a little bit of specific test coverage for this code.

Bug: 874592
Change-Id: I354c17a91751f930bd94d0ce9e02ef3f2846d15f
Reviewed-on: https://chromium-review.googlesource.com/c/1262655
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597196}
[modify] https://crrev.com/c6dfa55a3a5c70313bcd5c5cb736dd40d33f03bc/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/c6dfa55a3a5c70313bcd5c5cb736dd40d33f03bc/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/c6dfa55a3a5c70313bcd5c5cb736dd40d33f03bc/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/c6dfa55a3a5c70313bcd5c5cb736dd40d33f03bc/components/omnibox/browser/omnibox_edit_model_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 5

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

commit 388470daaff495f3639212e50682f5856f4b4340
Author: Tommy C. Li <tommycli@chromium.org>
Date: Fri Oct 05 22:09:29 2018

Omnibox: Query in Omnibox / Elisions - Page info bubble shows full URL.

After this CL, when the user opens the page info bubble (for instance
by clicking the lock icon), Chrome shows the full URL in the search bar.

This is a way for users to show the full underlying URL for both the
Steady State Elisions and Query in Omnibox features.

We are currently gating this behavior on the Query in Omnibox flag,
since it's still under active experimentation.

Bug: 874592
Change-Id: Icdb1b8ac79f6f8f3a90b9f82dcd838c5af6b767f
Reviewed-on: https://chromium-review.googlesource.com/c/1265497
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597335}
[modify] https://crrev.com/388470daaff495f3639212e50682f5856f4b4340/chrome/browser/ui/views/location_bar/location_bar_view.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 8

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

commit b9d74169d4da6045e7f4d1db87ae2b6d991f57f6
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Oct 08 17:21:55 2018

Omnibox: Query in Omnibox / Elisions - Reveal full URL for double Ctrl+L

After this CL, when the user presses Ctrl+L twice, we will reveal the
full URL.

Technically speaking, when the user issues a command to focus the
Omnibox while it's already focused, we will interpret it as a signal
that the user wants to see the full editable URL.

So it works for Alt+D and F6 as well.

This is gated behind the Query in Omnibox flag since it's still under
active experimentation.

Bug: 874592
Change-Id: I4d4a8944e4435a57be5e909f94ed65ff303be642
Reviewed-on: https://chromium-review.googlesource.com/c/1265825
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597594}
[modify] https://crrev.com/b9d74169d4da6045e7f4d1db87ae2b6d991f57f6/chrome/browser/ui/views/location_bar/location_bar_view.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 8

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

commit ba2e073e2c7964bdeb7e71129dd1f985b572a977
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Oct 08 18:00:11 2018

Omnibox: Query in Omnibox - Fix Cut/Copy of query to clipboard

Previously, the code was copying the full URL verbatim to the clipboard
when it saw that the user had not modified the display text at all.

This path was meant to gracefully handle the simple case where we elide
parts of the URL, but not meant for any Query in Omnibox case.

This CL disables that code path for Query in Omnibox, and adds a test.

Bug: 874592
Change-Id: I35dc12c55f906f078b87851803b97965ecacd6c8
Reviewed-on: https://chromium-review.googlesource.com/c/1259619
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597606}
[modify] https://crrev.com/ba2e073e2c7964bdeb7e71129dd1f985b572a977/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/ba2e073e2c7964bdeb7e71129dd1f985b572a977/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/ba2e073e2c7964bdeb7e71129dd1f985b572a977/components/omnibox/browser/omnibox_edit_model_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 8

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

commit 555e0ca1e1eff5f08f4380325f8bc84e6b805423
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Oct 08 18:19:13 2018

Omnibox: Query in Omnibox - Fix CurrentTextIsURL and match generation

This CL makes CurrentTextIsURL and the match generation mechanism
gracefully handle Query in Omnibox.

It also adds some tests for that.

Bug: 874592
Change-Id: I84e2bda6b489173a0a56ba2d6f461a1d0207fb11
Reviewed-on: https://chromium-review.googlesource.com/c/1263421
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597615}
[modify] https://crrev.com/555e0ca1e1eff5f08f4380325f8bc84e6b805423/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/555e0ca1e1eff5f08f4380325f8bc84e6b805423/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/555e0ca1e1eff5f08f4380325f8bc84e6b805423/components/omnibox/browser/omnibox_view_unittest.cc
[modify] https://crrev.com/555e0ca1e1eff5f08f4380325f8bc84e6b805423/components/omnibox/browser/test_omnibox_edit_model.cc
[modify] https://crrev.com/555e0ca1e1eff5f08f4380325f8bc84e6b805423/components/omnibox/browser/test_omnibox_edit_model.h

Hi tommycli@: Not sure if you are already working on, but I noticed that there is flicker in the Omnibox after pressing the Enter-key.

For example:

0.) enable chrome://flags/#enable-query-in-omnibox
1.) type chrome into the Omnibox
2.) press Enter

Actual: https://www.google.de/search?q=chrome&oq=chrome&aqs=chrome appears first in the Omnibox and then again "chrome".

Expected: Only showing "chrome" in the Omnibox without showing the search link https://www.google.de/search?q=chrome&oq=chrome&aqs=chrome.

Maybe the navigation link can be suppressed after pressing Enter?

I hope it is clear :) Otherwise I can attach a screencast. Please let me know.

Thanks.

Indeed you don't miss anything.

We are currently working on a fix to solve the problem on all platforms in a secure way:

https://chromium-review.googlesource.com/c/chromium/src/+/1274896
Thanks for the information :-)
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 29

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

commit 575dc09174cfeff0f04ac4b9b27733b584cfea57
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Oct 29 19:23:48 2018

Omnibox: Make a single Ctrl+L reveal the full URL

After this CL, a single Ctrl+L will reveal the full URL.

This applies to beth Query in Omnibox and Steady State Elisions.

This CL also adds a unit test, as well as an internal-only killswitch
in case we need to remotely disable this feature.

Bug: 874592, 882348
Change-Id: I6bce176e613b46bfc9b6ce524ddf855bdb14f399
Reviewed-on: https://chromium-review.googlesource.com/c/1285511
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603584}
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/components/omnibox/browser/omnibox_view.h
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/components/omnibox/browser/test_toolbar_model.cc
[modify] https://crrev.com/575dc09174cfeff0f04ac4b9b27733b584cfea57/components/omnibox/browser/test_toolbar_model.h

Is this supposed to work for all search providers or just Google? Currently I can see it working for Google but not for Bing.
kliffy542: It should work for your default search provider. Try setting your default search provider to Bing, and it should work for Bing searches.
Thanks! I am seeing this working, very neat!
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 12

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

commit e13a84b75aff2f23489b472f073ed2ba7e1d0a97
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Nov 12 19:10:29 2018

Omnibox: Expose IsSecurityInfoInitialized() method in LocationBarModel

For Query in Omnibox, we want to avoid the URL flicker while the page
is navigating and the TLS security state has not been initialized.

This currently works on Android implementation by toggling a boolean
flag on navigation and TLS state update.

However, this duplicates a piece of state that's already
authoritatively stored within the VisibleSecurityState.

This CL:

 1. Exposes an IsSecurityInfoInitialized() method on LocationBarModel

 2. Updates the security_state::SecurityInfo struct to add a
    connection_info_initialized flag that's already present in
    security_state::VisibleSecurityState.

 3. Updates LocationBarModelDelegate to provide a general
    GetSecurityInfo method.

Planned followup work:

 1. Make Query in Omnibox actually use the IsSecurityInfoInitialized
    flag.

 2. Use LocationBarModelDelegate::GetSecurityInfo to remove some
    now-redundant methods such as FailsBillingCheck and
    FailsMalwareCheck.

Bug: 874592
Change-Id: I648b30859ec40bb741de1c31378e0a6cd8baeeaa
Reviewed-on: https://chromium-review.googlesource.com/c/1324879
Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607296}
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/location_bar_model.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/location_bar_model_delegate.cc
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/location_bar_model_delegate.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/location_bar_model_impl.cc
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/location_bar_model_impl.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/test_location_bar_model.cc
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/omnibox/browser/test_location_bar_model.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/security_state/core/security_state.cc
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/components/security_state/core/security_state.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.h
[modify] https://crrev.com/e13a84b75aff2f23489b472f073ed2ba7e1d0a97/ios/chrome/browser/ui/location_bar/location_bar_model_delegate_ios.mm

Project Member

Comment 24 by bugdroid1@chromium.org, Nov 14

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

commit 3d82637f008ac74a3ae81c7fc9cf3a15638cf6ba
Author: Tommy C. Li <tommycli@chromium.org>
Date: Wed Nov 14 18:12:00 2018

Omnibox: Query in Omnibox - Remove Page Info unelision on Desktop

Removes the page info popup triggering unelision on Desktop.

Users found it more surprising than useful. We are adding other, more
explicit methods of unelision.

This CL is a simple revert of:
https://chromium-review.googlesource.com/c/chromium/src/+/1265497

Bug: 874592,  894862 
Change-Id: Ia1ebc7de91bb69f0aac9df22235b4ea24e7c0e93
Reviewed-on: https://chromium-review.googlesource.com/c/1332266
Reviewed-by: Kevin Bailey <krb@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608037}
[modify] https://crrev.com/3d82637f008ac74a3ae81c7fc9cf3a15638cf6ba/chrome/browser/ui/views/location_bar/location_bar_view.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 15

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

commit 4d10ddfc32cf7fab9c162deb6668a3074becb531
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Nov 15 18:09:00 2018

Omnibox: Query in Omnibox - Fix URL to query flicker on Desktop

On Desktop, with Query in Omnibox, the URL would flicker in for a second
while the search page was loading and before it got its security state.

This CL fixes that by ignoring the security state while the page is
loading. This is the same solution that the Android implementation has.

This CL also changes the ignore_security_state flag from a member
variable the caller toggles on and off to an additional parameter that's
passed in when the caller retrieves the query terms.

Bug: 874592
Change-Id: Ic5b8c78798b0119a63b5934c956a6395e4e6d769
Reviewed-on: https://chromium-review.googlesource.com/c/1274896
Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608435}
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/chrome/android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/chrome/android/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/chrome/browser/android/omnibox/query_in_omnibox_android.cc
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/chrome/browser/ui/android/toolbar/location_bar_model_android.cc
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/chrome/browser/ui/android/toolbar/location_bar_model_android.h
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/components/omnibox/browser/query_in_omnibox.cc
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/components/omnibox/browser/query_in_omnibox.h
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/components/omnibox/browser/query_in_omnibox_unittest.cc
[modify] https://crrev.com/4d10ddfc32cf7fab9c162deb6668a3074becb531/components/omnibox/browser/test_omnibox_client.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 3

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

commit 8a1133a0a03490e165640af6bc9dfd5d070c06ce
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Dec 03 19:55:50 2018

Omnibox: Make OmniboxView::GetIcon more robust for steady state elisions

Whenever the user unelides the URL, the omnibox also enters
user_input_in_progress_ mode.

This is was harmless before, but since then we have started displaying
suggestion favicons in the omnibox, the current page's security
indicator is being clobbered by the page's favicon during unelision.

This is not correct, since unelision is supposed to be as unobtrusive
as possible.

This is going to become more noticable now that Ctrl+L triggers
unelision, as well as our future planned work for One-Click-Unelide.

This CL replaces the old logic, and keeps showing the current page's
security indicator until the user actually modifies the user text.

Bug: 874592, 906223, 910145
Change-Id: I8a5619d4408b09d2e9f98fd5ce011ecbd2cab085
Reviewed-on: https://chromium-review.googlesource.com/c/1351564
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kevin Bailey <krb@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613203}
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/components/omnibox/browser/location_bar_model.h
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/components/omnibox/browser/location_bar_model_impl.cc
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/8a1133a0a03490e165640af6bc9dfd5d070c06ce/components/omnibox/browser/omnibox_view.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Dec 10

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

commit 580529891757508ee30404e36a2b187db2eedbfd
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Dec 10 21:38:08 2018

Omnibox Cross-platform UI: Fold QueryInOmnibox into LocationBarModel

Now that QueryInOmnibox is stateless (since we are now using
connection_info_initialized), it's logic is now small enough to fold
into LocationBarModel.

Previously, all the inputs of the function were sourced from
LocationBarModel anyways, so it's a natural fit.

Guide to reviewers:
 1. Moves everything from query_in_omnibox.h/cc to
    location_bar_model_impl.h/cc.
 2. Moves everything from query_in_omnibox_unittest.cc to
    location_bar_model_impl_unittest.cc.
 3. Some end-to-end testing code in LocationBarLayoutTest for
    Query in Omnibox removed in favor of our native unit tests.
    There's now only one test that tests the integration between
    LocationBarModel and the Android View.
 4. Everything else is mostly just plumbing.

This CL removes ~380 SLOC and two classes.

Bug: 874592, 912722
Change-Id: I1079d84a6b69a57410bc204ab8cedfdc5d27b3a3
Reviewed-on: https://chromium-review.googlesource.com/c/1366816
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615268}
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/android/java/src/org/chromium/chrome/browser/toolbar/LocationBarModel.java
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/android/java_sources.gni
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/BUILD.gn
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/browser/android/omnibox/query_in_omnibox_android.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/android/toolbar/location_bar_model_android.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/android/toolbar/location_bar_model_android.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/omnibox/chrome_omnibox_client.h
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/browser/ui/omnibox/query_in_omnibox_factory.cc
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/chrome/browser/ui/omnibox/query_in_omnibox_factory.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/toolbar/chrome_location_bar_model_delegate.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/BUILD.gn
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_delegate.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_delegate.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_impl.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_impl.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/location_bar_model_impl_unittest.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_client.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_client.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_edit_model.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_edit_model_unittest.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/omnibox_view.cc
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/components/omnibox/browser/query_in_omnibox.cc
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/components/omnibox/browser/query_in_omnibox.h
[delete] https://crrev.com/30a9458e826a1c73880e8a267a324392d2e74eaa/components/omnibox/browser/query_in_omnibox_unittest.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_location_bar_model.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_location_bar_model.h
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_omnibox_client.cc
[modify] https://crrev.com/580529891757508ee30404e36a2b187db2eedbfd/components/omnibox/browser/test_omnibox_client.h

Project Member

Comment 28 by bugdroid1@chromium.org, Dec 19

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

commit 1e89b8b85eceac02b4a662cbb23b5a8c03625ff7
Author: Tommy C. Li <tommycli@chromium.org>
Date: Wed Dec 19 05:56:32 2018

Omnibox: Query in Omnibox - Add Show URL context menu item.

This new Show URL item is shown whenever Query in Omnibox flag is
enabled and the current omnibox text is eligible for for unelision
(either when Query in Omnibox or Steady State Elisions is active).

This menu entry appears and goes away dynamically, and this required
some modification to Textfield. However, I think this is overall a
good modification, as the old system was a bit misleading.

Bug: 874592
Change-Id: Ife83e6f575745a33e96d7e0da715199ea794df77
Reviewed-on: https://chromium-review.googlesource.com/c/1279353
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617737}
[modify] https://crrev.com/1e89b8b85eceac02b4a662cbb23b5a8c03625ff7/chrome/app/generated_resources.grd
[add] https://crrev.com/1e89b8b85eceac02b4a662cbb23b5a8c03625ff7/chrome/app/generated_resources_grd/IDS_SHOW_URL.png.sha1
[modify] https://crrev.com/1e89b8b85eceac02b4a662cbb23b5a8c03625ff7/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/1e89b8b85eceac02b4a662cbb23b5a8c03625ff7/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/1e89b8b85eceac02b4a662cbb23b5a8c03625ff7/ui/views/controls/textfield/textfield_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 21

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

commit 4a6dd400e71c6f0e4e2489a49b041b90b37ba31b
Author: Tommy C. Li <tommycli@chromium.org>
Date: Fri Dec 21 23:22:20 2018

Omnibox Code Health: Port 4 clipboard browser tests to unit tests

Doing this work for a couple reasons:
 - Unit tests are less flaky and faster.
 - These can be written as unit tests without losing any testing power.
 - Query in Omnibox may need to update the clipboard behavior soon,
   this is kind of adjacent work.

Bug: 874592, 751031
Change-Id: Ibf5273f22578be713ab3c48539ab5311e5c7a7cc
Reviewed-on: https://chromium-review.googlesource.com/c/1377174
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618663}
[modify] https://crrev.com/4a6dd400e71c6f0e4e2489a49b041b90b37ba31b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
[modify] https://crrev.com/4a6dd400e71c6f0e4e2489a49b041b90b37ba31b/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jan 7

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

commit 6cf03f63450bbce6688129fb181b773616967342
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Jan 07 18:38:44 2019

Omnibox: Respect |write_url| in AdjustTextForCopy for OmniboxViewViews

This CL makes the OmniboxViewViews respect the |write_url| parameter
returned by OmniboxEditModel::AdjustTextForCopy.

This also makes copied Query in Omnibox terms interpreted as a
hyperlink.

It is also copied as plain text additionally, so it should continue to
work in plain-text contexts.

This has a few advantages:

 1. Makes Query in Omnibox copied queries more useful, as it's now a
    hyperlink that can be pasted into documents.

 2. Makes copied URLs immediately hyperlinks within word processing
    apps, without relying on their autoformat.

 3. Makes Desktop (Views) Chrome more congruent with iOS behavior.

Bug: 874592
Change-Id: I95b835b21d1dbe22de7a4acbf82fda7834bf4699
Reviewed-on: https://chromium-review.googlesource.com/c/1370909
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620395}
[modify] https://crrev.com/6cf03f63450bbce6688129fb181b773616967342/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
[modify] https://crrev.com/6cf03f63450bbce6688129fb181b773616967342/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc
[modify] https://crrev.com/6cf03f63450bbce6688129fb181b773616967342/components/omnibox/browser/omnibox_edit_model.cc
[modify] https://crrev.com/6cf03f63450bbce6688129fb181b773616967342/components/omnibox/browser/omnibox_edit_model_unittest.cc

Sign in to add a comment