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

Issue 773643 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Pull To Action (PTA) needs to adapt to the different height of toolbars.

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

Issue description

Right now, PTA 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) Visit a website (e.g. wikipedia).
(4) Trigger PTA (e.g. do a refresh with PTA). Notice that the PTA ui uses the correct height.
(5) Change orientation.
(6) Trigger PTA. Notice that the PTA ui uses an incorrect height.
 

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

Owner: jif@chromium.org
Status: Started (was: Available)

Comment 3 by jif@chromium.org, Oct 13 2017

Labels: Hotlist-iOS11 Hotlist-iPhoneX M-63
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 13 2017

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

commit 9053173ab0cfd184114ed9dab0294e6865cf458b
Author: Jean-François Geyelin <jif@chromium.org>
Date: Fri Oct 13 17:35:55 2017

[iOS] Update the initial header height at every Pull To Action start.

Before this CL, the "initial header height" was only set when the
CRWWebController changed.
Since the iPhone X, the header height can change when the orientation
changes.

With this CL, the header height is set whenever the Pull To Action
flow starts.
Note that an on-going Pull To Action is cancelled if the screen is rotated,
so the  header height can't change in the middle of a Pull To Action.


Bug:  773643 
Change-Id: I6891d5da8de610d8cbfcab0f880c6f807ec98df5
Reviewed-on: https://chromium-review.googlesource.com/718099
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Jean-François Geyelin <jif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508742}
[modify] https://crrev.com/9053173ab0cfd184114ed9dab0294e6865cf458b/ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.mm

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

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

Comment 6 by sheriffbot@chromium.org, Oct 17 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 20 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by sheriffbot@chromium.org, Oct 23 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 27 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16424484351b68c1a4a3bc67743f26a5b28f9999

commit 16424484351b68c1a4a3bc67743f26a5b28f9999
Author: Jean-François Geyelin <jif@chromium.org>
Date: Fri Oct 27 14:13:19 2017

[iOS] Update the initial header height at every Pull To Action start.

Before this CL, the "initial header height" was only set when the
CRWWebController changed.
Since the iPhone X, the header height can change when the orientation
changes.

With this CL, the header height is set whenever the Pull To Action
flow starts.
Note that an on-going Pull To Action is cancelled if the screen is rotated,
so the  header height can't change in the middle of a Pull To Action.


Bug:  773643 
Change-Id: I6891d5da8de610d8cbfcab0f880c6f807ec98df5
Reviewed-on: https://chromium-review.googlesource.com/718099
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Jean-François Geyelin <jif@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#508742}(cherry picked from commit 9053173ab0cfd184114ed9dab0294e6865cf458b)
Reviewed-on: https://chromium-review.googlesource.com/741503
Reviewed-by: Jean-François Geyelin <jif@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#261}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/16424484351b68c1a4a3bc67743f26a5b28f9999/ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.mm

Sign in to add a comment