New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 867458 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 867455



Sign in to add a comment

Fake omnibox not removed when focused from bottom toolbar

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

Issue description

M70

What steps will reproduce the problem?
(1) Open NTP
(2) Scroll the NTP such as the fake omnibox is pinned to the top
(3) Tap the search button on the bottom toolbar

What is the expected result?
The fake omnibox should be focused and replaced with the real omnibox

What happens instead?
The fake omnibox isn't removed, but the real omnibox is focus. It is possible to type but nothing appear on screen.

p1 as it might be linked to  issue 865408 
 
Blockedon: 867455
Holding off on this as we may revert some of the ntp focus changes, and this will be fixed by that.  Otherwise it's an easy 1 line fix.  But for now it's blocked by  crbug.com/867455 
Status: Started (was: Assigned)
No longer blocked, I'll fix here crrev.com/c/1150427
Issue 867560 has been merged into this issue.
Cc: bkawaichi@chromium.org gambard@chromium.org justincohen@chromium.org martijnb@chromium.org marq@chromium.org stkhapugin@chromium.org
 Issue 867691  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 26

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

commit 96585bf6ff840727b7e1238b59ffb5a9f89840c6
Author: Justin Cohen <justincohen@google.com>
Date: Thu Jul 26 12:09:46 2018

[ios] Fix broken NTP when focusing the omnibox from the toolbar.

Always call shiftTilesUp on both refresh and non-UI Refresh, but
don't stomp on |collectionShiftingOffset| if the header is
already pinned.

Bug:  867458 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I21f805eeca85406db5389c8b89db59fd097b4a10
Reviewed-on: https://chromium-review.googlesource.com/1150427
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578267}
[modify] https://crrev.com/96585bf6ff840727b7e1238b59ffb5a9f89840c6/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_synchronizer.mm
[modify] https://crrev.com/96585bf6ff840727b7e1238b59ffb5a9f89840c6/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.mm
[modify] https://crrev.com/96585bf6ff840727b7e1238b59ffb5a9f89840c6/ios/chrome/browser/ui/content_suggestions/ntp_home_egtest.mm

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

Comment 8 by sheriffbot@chromium.org, Jul 27

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
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 27

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

commit e424c1615c1f97c7bc4db3c378b433435cbf0d11
Author: Justin Cohen <justincohen@google.com>
Date: Fri Jul 27 17:44:38 2018

[ios] Fix broken NTP when focusing the omnibox from the toolbar.

Always call shiftTilesUp on both refresh and non-UI Refresh, but
don't stomp on |collectionShiftingOffset| if the header is
already pinned.

Bug:  867458 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I21f805eeca85406db5389c8b89db59fd097b4a10
Reviewed-on: https://chromium-review.googlesource.com/1150427
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578267}(cherry picked from commit 96585bf6ff840727b7e1238b59ffb5a9f89840c6)
Reviewed-on: https://chromium-review.googlesource.com/1153407
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#165}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/e424c1615c1f97c7bc4db3c378b433435cbf0d11/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_synchronizer.mm
[modify] https://crrev.com/e424c1615c1f97c7bc4db3c378b433435cbf0d11/ios/chrome/browser/ui/content_suggestions/content_suggestions_header_view_controller.mm
[modify] https://crrev.com/e424c1615c1f97c7bc4db3c378b433435cbf0d11/ios/chrome/browser/ui/content_suggestions/ntp_home_egtest.mm

Status: Verified (was: Fixed)
Verified on chrome canary version 70.0.3508.0 on iPhone 8 plus with iOS 11.4.1, 12 beta  following steps mentioned in comment #0.  Looks good.
Verified in:

App Version: 69.0.3497.22 beta
Devices: iPhone 7 Plus, iPhone 6 Plus
iOS Version: 10.3.3, 11.4.1

Fake omnibox is focused when focusing the omnibox from toolbar

Sign in to add a comment