New issue
Advanced search Search tips

Issue 789471 link

Starred by 1 user

Issue metadata

Status: Verified
Merged: issue 792503
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

The "clear omnibox" button is overlapping with the "close omnibox" button

Project Member Reported by gambard@chromium.org, Nov 29 2017

Issue description

Chrome Version: M64

What steps will reproduce the problem?
(1) Open a new incognito tab
(2) Navigate to a page
(3) Tap the omnibox

What is the expected result?
The clear and close omnibox buttons should be separated

What happens instead?
The two buttons are on top of each others

Might be related to issue 788814

Please assess if RBS.
 
Simulator Screen Shot - iPhone 8 Plus - 2017-11-29 at 11.19.53.png
279 KB View Download
Labels: M-64 ReleaseBlock-Stable
Cc: gambard@chromium.org stkhapugin@chromium.org justincohen@chromium.org
Components: UI>Browser>Toolbar
Owner: sczs@chromium.org
The issue is that WTC doesn't position the LocationBar correctly.

This seems to be broken by the changes in WTC related to expansion animations.
The issue is not reproducible when the property animator flag is on (or when the new toolbar flag is on). 
Please note that this is an M64 RBS, so the old animation code needs to be fixed. 
Cc: -stkhapugin@chromium.org sczs@chromium.org
Owner: stkhapugin@chromium.org

Comment 4 by sczs@chromium.org, Dec 7 2017

I briefly started to take a look at this yesterday night.
I couldn't repro on 64.0.3254.0 dev. I also realized that the Omnibox expands to position itself underneath the WTC (this happens on iPad) so the WTC containment is not a sure thing.
I also checked that the new animation code didn't make any changes besides adding code behind flags. (Though maybe a follow up CL could've change something).

stk@ are you going to continue working on this? Do you think is omnibox related?
Mergedinto: 792503
Status: Duplicate (was: Assigned)
sczs@ please follow along here: 792503
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 8 2017

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

commit f2dfa460b70841647d1b5dcb7700fd0fd614737c
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Fri Dec 08 15:30:21 2017

Set target location bar view frame to trigger autolayout.

Set the view frame in addition to animating view's layer bounds because
just changing layer bounds doesn't allow autolayout inside LBV to
trigger.
Note that after [_locationBarView layer].bounds = toBounds; the
LBV's frame will become LayoutRectGetRect(toLayout) anyway, but the
since the setter is not called, autolayout seems to be confused.
Also note that this is not necessary in new toolbar.

Bug:  792503 ,  789471 , 788814
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If3ccac317bb8d850f71275ee32c7b82d38f6af24
Reviewed-on: https://chromium-review.googlesource.com/813973
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522784}
[modify] https://crrev.com/f2dfa460b70841647d1b5dcb7700fd0fd614737c/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Status: Verified (was: Duplicate)
Verified on M65.0.3292.0 Canary on iPhone X iOS 11.1, iPhone 7+ iOS 10.3.3, iPad Pro 12'5 iOS 11.2 in Incognito tab.
Labels: Merge-Request-64
Thanks vbhatsoori@! Requesting cherry-pick to M64. 

Comment 9 by cma...@chromium.org, Dec 13 2017

Labels: -Merge-Request-64 Merge-Approved-64
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 18 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/843d606a97a222a5ff7556fffb9a1f3eeeab5ca2

commit 843d606a97a222a5ff7556fffb9a1f3eeeab5ca2
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Mon Dec 18 15:07:14 2017

Set target location bar view frame to trigger autolayout.

Set the view frame in addition to animating view's layer bounds because
just changing layer bounds doesn't allow autolayout inside LBV to
trigger.
Note that after [_locationBarView layer].bounds = toBounds; the
LBV's frame will become LayoutRectGetRect(toLayout) anyway, but the
since the setter is not called, autolayout seems to be confused.
Also note that this is not necessary in new toolbar.

Bug:  792503 ,  789471 , 788814
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If3ccac317bb8d850f71275ee32c7b82d38f6af24
Reviewed-on: https://chromium-review.googlesource.com/813973
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522784}(cherry picked from commit f2dfa460b70841647d1b5dcb7700fd0fd614737c)
Reviewed-on: https://chromium-review.googlesource.com/832646
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#262}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/843d606a97a222a5ff7556fffb9a1f3eeeab5ca2/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Verified in:

App Version: 64.0.3282.41 beta
Devices: iPhone 6S Plus, iPhone 8 Plus
iOS Versions: 10.3.3, 11.2.5 beta 2

Clear Omnibox button is not overlapping with the Close button in Omnibox. Issue is fixed.

Sign in to add a comment