New issue
Advanced search Search tips

Issue 865000 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug
Q3



Sign in to add a comment

RxR leading edge completion icon in omnibox wrong for some states

Project Member Reported by pschaffner@chromium.org, Jul 18

Issue description

In RxR, the leading edge completion icon in the omnibox gets out of sync sometimes. To repro:

1) focus the omnibox and note the (i) icon (which should be the generic "globe" icon).
2) create a new tab and focus the omnibox an see the same (i) icon (which should be a magnifying glass)

Note that if after step #1, you type some characters to get a different completion icon in the omnibox, then directly create a new tab and focus the ominbox, you will see that different icon from the previously focused omnibox.
 
Cc: -stkhapugin@chromium.org justincohen@chromium.org
Owner: stkhapugin@chromium.org
Status: Assigned (was: Untriaged)
stkhapugin@ PTAL
Labels: Q3 M-69 MS-Omnibox
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 19

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

commit 4b44f6abd5086b3f3d79888b07f7bad3184772ab
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Thu Jul 19 14:10:23 2018

Pass topmost suggestion type instead of image id when image changes.

Moves the logic to retrieve the image for the topmost suggestion in the
omnibox from omnibox_popup_view_ios to the left image consumer.

Bug:  865000 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I1b839bb546d5aa652e7db87c91fe88fcbc2b594e
Reviewed-on: https://chromium-review.googlesource.com/1143184
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576476}
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/location_bar/location_bar_legacy_view.mm
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/omnibox/omnibox_left_image_consumer.h
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/omnibox/omnibox_mediator.mm
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/omnibox/omnibox_view_ios.h
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_view_ios.mm
[modify] https://crrev.com/4b44f6abd5086b3f3d79888b07f7bad3184772ab/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_view_suggestions_delegate.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 25

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

commit 92983774c5451d021d409481c20ca2cfab14948d
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Wed Jul 25 14:17:23 2018

Implement default leading image behaviour in the omnibox.

Adds a default leading image to the omnibox to be used when
it's being focused (before the first update of the leading image).
There are two images: for the empty textfield (magnifying glass)
and the non-empty textfield (default favicon).

Bug:  865000 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib9869c3b87f81041ec8a13b0ac220a3b93aa521e
Reviewed-on: https://chromium-review.googlesource.com/1146926
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577877}
[modify] https://crrev.com/92983774c5451d021d409481c20ca2cfab14948d/ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm
[modify] https://crrev.com/92983774c5451d021d409481c20ca2cfab14948d/ios/chrome/browser/ui/omnibox/omnibox_view_controller.h
[modify] https://crrev.com/92983774c5451d021d409481c20ca2cfab14948d/ios/chrome/browser/ui/omnibox/omnibox_view_controller.mm

Status: Fixed (was: Started)
CL 1146926 will need to be merged after verification. 
Cc: stkhapugin@chromium.org
 Issue 865027  has been merged into this issue.
Status: Verified (was: Fixed)
Search icon is displayed when tapping on Omnibox.
Search suggestions also showing a magnifying glass.

Verified on M70.0.3508.0 dev
iOS: 11.4.1, iPad Pro 9"
Labels: Merge-Request-69
Project Member

Comment 9 by sheriffbot@chromium.org, Aug 2

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Manually approved.
Project Member

Comment 11 by sheriffbot@chromium.org, Aug 6

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 8

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8477b8668c806778d28555fbc99b8a44547875fa

commit 8477b8668c806778d28555fbc99b8a44547875fa
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Wed Aug 08 09:48:42 2018

Implement default leading image behaviour in the omnibox.

Adds a default leading image to the omnibox to be used when
it's being focused (before the first update of the leading image).
There are two images: for the empty textfield (magnifying glass)
and the non-empty textfield (default favicon).

TBR=stkhapugin@chromium.org

(cherry picked from commit 92983774c5451d021d409481c20ca2cfab14948d)

Bug:  865000 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib9869c3b87f81041ec8a13b0ac220a3b93aa521e
Reviewed-on: https://chromium-review.googlesource.com/1146926
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577877}
Reviewed-on: https://chromium-review.googlesource.com/1166912
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#490}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/8477b8668c806778d28555fbc99b8a44547875fa/ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm
[modify] https://crrev.com/8477b8668c806778d28555fbc99b8a44547875fa/ios/chrome/browser/ui/omnibox/omnibox_view_controller.h
[modify] https://crrev.com/8477b8668c806778d28555fbc99b8a44547875fa/ios/chrome/browser/ui/omnibox/omnibox_view_controller.mm

Search icon is displayed when tapping on Omnibox.
Search suggestions also showing a magnifying glass.

Verified on M69.0.3497.41 beta
Device: iPad Pro
iOS 11.4.1

Sign in to add a comment