New issue
Advanced search Search tips

Issue 866882 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug
Q2



Sign in to add a comment

URLs blink on tapping in omnibox

Project Member Reported by vbarig...@chromium.org, Jul 24

Issue description

App Version: 69.0.3497.7 beta
iOS Version: 11.4.1, 12 beta 4
Device: iPhones only
URL: rediff.com, yahoo.com

Steps to reproduce:
  1.  Launch chrome canary
  2.  Type to rediff.com in omnibox
  3.  Notice that suggestions are displayed
  4.  Scroll up the suggestions.
  5.  Tap in Omnibox
 
Observed results:
Notice that the URL blinks for moment

Expected results:
URL should not blink.

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: NA
Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA
Bug reproducible on current stable build (App Version, iOS Version): NA on M67
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M69, M70

Link to video/image:  https://drive.google.com/file/d/1gVRmYLrPjeSsgQTvcbkO_2HamLTPxTTN/view?usp=sharing

 
Labels: -found-in-m96 found-in-M69
Labels: -Pri-2 M-69 Q2 Pri-1
Owner: stkhapugin@chromium.org
Status: Assigned (was: Untriaged)
This seems pretty prominent.
Cc: rohitrao@chromium.org
Status: Started (was: Assigned)
This is a really interesting bug. It's hard to solve because the popup is considered open when there are autocomplete suggestions, and there are autocomplete suggestions BEFORE the BVC knows that omnibox is getting focused. The only place that knows that the popup was not shown when the focus even happens is OmniboxViewIOS. 

Luckily, WebOmniboxEditControllerImpl (the LocationBarControllerImpl replacement in UI Refresh) doesn't do anything in OnSetFocus() except forwarding the call to the delegate (BVC). The BVC code only really needs to happen when the popup is appearing for the first time: OnKillFocus() is not called when the popup is still displayed, so BVC does not undo things it does from locationBarHasBecomeFirstResponder until the popup is dismissed. So it actually makes more sense to not call OnSetFocus() multiple times, and it might had been a mistake to let this happen. However I think we should keep old behaviour for the pre-UI Refresh omnibox, just to be on the safe side. 

Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/1151347.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1

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

commit c45bd761564983475deaff4b09a055b854c228fb
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Wed Aug 01 09:31:26 2018

[iOS] Prevent omnibox focusing animation when the popup is up.

Prevents the focusing animation when the popup is displayed.
Also resets alpha levels of the edit and steady view post animation to
ensure correct behaviour if the non-animated focusing/defocusing is
triggered multiple times rapidly (more of a theoretical scenario).

Bug:  866882 
TBR: marq@chromium.org
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib00aa7d4a5ffe871260b4272ad9b2ca8494ba952
Reviewed-on: https://chromium-review.googlesource.com/1151347
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579737}
[modify] https://crrev.com/c45bd761564983475deaff4b09a055b854c228fb/ios/chrome/browser/ui/location_bar_notification_names.h
[modify] https://crrev.com/c45bd761564983475deaff4b09a055b854c228fb/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
[modify] https://crrev.com/c45bd761564983475deaff4b09a055b854c228fb/ios/chrome/browser/ui/omnibox/web_omnibox_edit_controller.h
[modify] https://crrev.com/c45bd761564983475deaff4b09a055b854c228fb/ios/chrome/browser/ui/orchestrator/omnibox_focus_orchestrator.mm

Status: Fixed (was: Started)
Labels: Merge-TBD
Labels: -Merge-TBD Merge-Request-69
Status: Verified (was: Fixed)
Project Member

Comment 8 by sheriffbot@chromium.org, Aug 6

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Less than 25 days to go before AppStore submit on M69
Please contact the 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
Labels: -Merge-Review-69 Merge-Approved-69
Approved. Please merge asap.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 6

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

commit 5b098ff87414ad6961109b99870cd426feecbb4a
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Mon Aug 06 14:11:25 2018

[iOS] Prevent omnibox focusing animation when the popup is up.

Prevents the focusing animation when the popup is displayed.
Also resets alpha levels of the edit and steady view post animation to
ensure correct behaviour if the non-animated focusing/defocusing is
triggered multiple times rapidly (more of a theoretical scenario).

Bug:  866882 
TBR: marq@chromium.org
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib00aa7d4a5ffe871260b4272ad9b2ca8494ba952
Reviewed-on: https://chromium-review.googlesource.com/1151347
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579737}(cherry picked from commit c45bd761564983475deaff4b09a055b854c228fb)
Reviewed-on: https://chromium-review.googlesource.com/1161886
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#414}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/5b098ff87414ad6961109b99870cd426feecbb4a/ios/chrome/browser/ui/location_bar_notification_names.h
[modify] https://crrev.com/5b098ff87414ad6961109b99870cd426feecbb4a/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
[modify] https://crrev.com/5b098ff87414ad6961109b99870cd426feecbb4a/ios/chrome/browser/ui/omnibox/web_omnibox_edit_controller.h
[modify] https://crrev.com/5b098ff87414ad6961109b99870cd426feecbb4a/ios/chrome/browser/ui/orchestrator/omnibox_focus_orchestrator.mm

Verified on M69.0.3497.31 beta
iOS: 10.3.3, 12.0 beta#6
iPhone7Plus, iPhoneX

Sign in to add a comment