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

Issue 774969 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Swipe the toolbar to change tabs needs to adapt to the different height of toolbars.

Project Member Reported by jif@chromium.org, Oct 16 2017

Issue description

Right now, Swipe to change tab considers the height of the toolbar to be fixed.
This is a problem on the iPhone X, where the height of the toolbar changes with the orientation.

To reproduce:
(1) On iPhone X, enable the "Safe area compatible toolbar" experimental flag.
(2) Relaunch chrome.
(3) Open 2 tabs each containing a website (e.g. wikipedia).
(4) Swipe between the 2 tabs. Notice the toolbar looks OK during the swiping.
(5) Change orientation.
(6) Swipe between the 2 tabs. Notice the toolbar is stretched.

 

Comment 1 by jif@chromium.org, Oct 16 2017

Screen Shot 2017-10-16 at 11.44.52.png
173 KB View Download

Comment 2 by jif@chromium.org, Oct 16 2017

Owner: lod@chromium.org
Status: Assigned (was: Available)
Components: UI>Browser>Toolbar

Comment 4 by jif@chromium.org, Oct 24 2017

Labels: -Pri-3 Pri-1

Comment 5 by jif@chromium.org, Oct 24 2017

Labels: ReleaseBlock-Stable

Comment 6 by lod@chromium.org, Oct 27 2017

Status: Started (was: Assigned)

Comment 7 by cma...@chromium.org, Oct 31 2017

lod@ Any update on this RBS?

Comment 8 by jif@chromium.org, Oct 31 2017

May not be RBS anymore as we may not enable the "Safe area compatible toolbar" flag in M63.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 6 2017

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

commit edd14e0644b2580407ef5ddabb07af6c0c7a7e43
Author: Elodie Banel <lod@google.com>
Date: Mon Nov 06 14:16:05 2017

Adapt toolbar in swipeview to changing status bar heights.

Bug:  774969 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I483d94ad34af8e799b4560c15ab225eabf8659a4
Reviewed-on: https://chromium-review.googlesource.com/754683
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Elodie Banel <lod@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514125}
[modify] https://crrev.com/edd14e0644b2580407ef5ddabb07af6c0c7a7e43/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm

Comment 10 by lod@chromium.org, Nov 6 2017

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-63; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-63 label, otherwise remove Merge-TBD label. Thanks.
Cc: pkl@chromium.org justincohen@chromium.org
is this something we want to merge into 63?

Comment 13 by pkl@chromium.org, Nov 13 2017

Probably not, since this happens only with SACT flag enabled.
lod: Please confirm and modify Merge-TBD label accordingly.
Status: Verified (was: Fixed)
Tested in latest build today, the issue - "Swipe the toolbar to change tabs needs to adapt to the different height of toolbars." is fixed.  The toolbar height is fixed now.

Build tested: 64.0.3267.0 Canary
iOS: 11.1
Devices: iPhone X
Removing Merge-TBD since this was not supposed to be merged into M63 given there was no confirmation from Elodie and comment 8
Labels: -Merge-TBD

Sign in to add a comment