Issue metadata
Sign in to add a comment
|
The "clear omnibox" button is overlapping with the "close omnibox" button |
||||||||||||||||||||||||
Issue descriptionChrome 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.
,
Dec 5 2017
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.
,
Dec 7 2017
,
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?
,
Dec 7 2017
,
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
,
Dec 12 2017
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.
,
Dec 13 2017
Thanks vbhatsoori@! Requesting cherry-pick to M64.
,
Dec 13 2017
,
Dec 18 2017
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
,
Dec 20 2017
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 |
|||||||||||||||||||||||||
Comment 1 by justincohen@chromium.org
, Dec 5 2017