New issue
Advanced search Search tips

Issue 867456 link

Starred by 1 user
Status: Verified
Owner:
justincohen@chromium.org
Closed: Sep 12
Cc:
mard...@chromium.org
pschaffner@chromium.org
gambard@chromium.org
srikanthg@chromium.org
ios-bugs@chromium.org
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 

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

Comment 1 by justincohen@chromium.org, Jul 25

Cc: srikanthg@chromium.org
This is how stable looks, pretty sure this is a dup.

Comment 2 by gambard@chromium.org, Jul 25

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.

Comment 3 by srikanthg@chromium.org, Jul 25 

http://crbug/821593 comment#9 is for UI-Refresh.

Comment 4 by justincohen@chromium.org, Jul 25 

The video in 821593 looks identical. gambard@ what workaround are you referring to?

Comment 5 by gambard@chromium.org, Jul 25 

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.

Comment 6 by justincohen@chromium.org, Jul 25 

What is the workaround?

Comment 7 by gambard@chromium.org, Jul 25 

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.

Comment 8 by fgor...@chromium.org, Jul 31

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

Comment 10 by justincohen@chromium.org, Sep 12

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

Comment 12 by vbarig...@chromium.org, Sep 18

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

Comment 14 by vbarig...@chromium.org, Sep 19 

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