New issue
Advanced search Search tips

Issue 867456 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Copy to Clipboard - NTP - Fakebox focus - animation

Project Member Reported by martijnb@chromium.org, Jul 25

Issue description


What steps will reproduce the problem?
(1) Copy URL to clipboard
(2) Go to NTP
(3) Tap magnify icon in bottom toolbar

What is the expected result?

What happens instead?
The animation towards a focused Fakebox is buggy, e.g the Google icon flies out. 
See Video. 
 
ScreenRecording_07-25-2018 14-29-09.MP4
3.5 MB View Download
Cc: srikanthg@chromium.org
This is how stable looks, pretty sure this is a dup.
Status: Assigned (was: Untriaged)
This only happens with UI Refresh as there is a workaround for this when you are tapping the fake omnibox. We can't do it when the omnibox is focused using the bottom toolbar.
So I don't think it is a dup.
http://crbug/821593 comment#9 is for UI-Refresh.
The video in 821593 looks identical. gambard@ what workaround are you referring to?


Yes it is the same issue actually. I didn't remember that the UI refresh was added.
There is a workaround when you tap the fake omnibox.
What is the workaround?
There is a -focusFakebox command (FakeboxFocuser protocol) which shows the real omnibox directly. However, looking at the code this seems to be for non-UI Refresh only.
Labels: zine-triaged
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 12

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

commit 778134b4bb7722f4227ae90a5348019b38999f03
Author: Justin Cohen <justincohen@google.com>
Date: Wed Sep 12 13:49:06 2018

[ios] Correct NTP focus animation when focusing from outside the NTP.

Bug:  867456 , 839385
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I693a356546163519c07e0e60f090e0cfeec0413b
Reviewed-on: https://chromium-review.googlesource.com/1198310
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590659}
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/commands/browser_commands.h
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.h
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/location_bar/BUILD.gn
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/ntp/new_tab_page_controller.h
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/toolbar/public/fakebox_focuser.h
[modify] https://crrev.com/778134b4bb7722f4227ae90a5348019b38999f03/ios/chrome/browser/ui/toolbar/public/omnibox_focuser.h

Status: Fixed (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 13

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/992822694a7c204f5ea3739146627d5fa64d588a

commit 992822694a7c204f5ea3739146627d5fa64d588a
Author: Justin Cohen <justincohen@google.com>
Date: Thu Sep 13 16:10:50 2018

[ios] Correct NTP focus animation when focusing from outside the NTP.

Bug:  867456 , 839385, 821593
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I693a356546163519c07e0e60f090e0cfeec0413b
Reviewed-on: https://chromium-review.googlesource.com/1198310
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590659}(cherry picked from commit 778134b4bb7722f4227ae90a5348019b38999f03)
Reviewed-on: https://chromium-review.googlesource.com/1224672
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#368}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/commands/browser_commands.h
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.h
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/location_bar/BUILD.gn
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/ntp/new_tab_page_controller.h
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/ntp/new_tab_page_toolbar_controller.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/toolbar/public/fakebox_focuser.h
[modify] https://crrev.com/992822694a7c204f5ea3739146627d5fa64d588a/ios/chrome/browser/ui/toolbar/public/omnibox_focuser.h

Status: Verified (was: Fixed)
Verified on chrome canary version 71.0.3550.0 on iPhone 8 plus with iOS 12 and iPhone 6 plus with iOS 11.4.1, following the steps mentioned in comment #0.  Animation is smooth.  Looks good.

Comment 13 Deleted

Verified on chrome beta version 70.0.3538.22 on iPhone 8 plus and iPhone 6 with iOS 11.4.1 and  iPhone 6 plus with iOS 12 GM , following the steps mentioned in comment #0.  Animation is smooth on tapping fakebox.  Looks good.

Sign in to add a comment