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

Issue 767869 link

Starred by 1 user
Status: Verified
Owner:
gambard@chromium.org
Closed: Sep 2017
Cc:
ios-bugs-priority@chromium.org
ios-bugs@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Bottom toolbar of NTP should respect safe area on iPhone X

Project Member Reported by gambard@chromium.org, Sep 22 2017 
iPhone X only

What steps will reproduce the problem?
(1) Open NTP

What is the expected result?
The bottom toolbar allowing to select bookmarks or recent tabs should respect the safe area.

What happens instead?
The toolbar is displayed inside the safe area.
Simulator Screen Shot - iPhone X - 2017-09-22 at 15.50.19.png
481 KB View Download

Comment 1 by fi...@chromium.org, Sep 25 2017

Labels: zine-triaged
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28 2017 

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

commit d909eb8729040c2d81b8953312c274245d97031a
Author: Gauthier Ambard <gambard@chromium.org>
Date: Thu Sep 28 16:03:53 2017

Respect safe area for NTP view

The NTP view cannot use the safeAreaInsets property directly as it is
snapshotted before being inserted in the view hierarchy, and the safe
area is only available for views in the view hierarchy.
This CL introduces a property mirroring the safe area once the view is
in the view hierarchy. But it is also set up with the value used in the
first frame, allowing the snapshot to be done as if the view was in the
view hierarchy.

Bug:  767869 
Change-Id: I605566105862e9f4c557736ec65df5f8ab1fc76b
Reviewed-on: https://chromium-review.googlesource.com/681942
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Reviewed-by: Jean-François Geyelin <jif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505041}
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_bar.h
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_bar.mm
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_controller.h
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_view.h
[modify] https://crrev.com/d909eb8729040c2d81b8953312c274245d97031a/ios/chrome/browser/ui/ntp/new_tab_page_view.mm

Comment 3 by gambard@chromium.org, Sep 28 2017

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 28 2017 

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

commit 47b872e6f52e5bc4cf8d316f44f916a1c295656a
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Thu Sep 28 19:29:17 2017

Revert "Respect safe area for NTP view"

This reverts commit d909eb8729040c2d81b8953312c274245d97031a.

Reason for revert: This CL is causing RecentTabsTableTestCase/testClosedTabAppearsInRecentTabsPanel to fail on iPad: https://build.chromium.org/p/chromium.fyi/builders/EarlGreyiOS/builds/31975

Original change's description:
> Respect safe area for NTP view
> 
> The NTP view cannot use the safeAreaInsets property directly as it is
> snapshotted before being inserted in the view hierarchy, and the safe
> area is only available for views in the view hierarchy.
> This CL introduces a property mirroring the safe area once the view is
> in the view hierarchy. But it is also set up with the value used in the
> first frame, allowing the snapshot to be done as if the view was in the
> view hierarchy.
> 
> Bug:  767869 
> Change-Id: I605566105862e9f4c557736ec65df5f8ab1fc76b
> Reviewed-on: https://chromium-review.googlesource.com/681942
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
> Reviewed-by: Jean-François Geyelin <jif@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#505041}

TBR=rohitrao@chromium.org,jif@chromium.org,gambard@chromium.org

Change-Id: I05bb5751f1ec6aec184dfe6ccff7741f2df4182d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  767869 
Reviewed-on: https://chromium-review.googlesource.com/690817
Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505118}
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_bar.h
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_bar.mm
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_controller.h
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_view.h
[modify] https://crrev.com/47b872e6f52e5bc4cf8d316f44f916a1c295656a/ios/chrome/browser/ui/ntp/new_tab_page_view.mm
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 29 2017 

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

commit d889045ed5be75ca9eb5301459ce29dc03df0e69
Author: Gauthier Ambard <gambard@chromium.org>
Date: Fri Sep 29 12:07:46 2017

Respect safe area for NTP view

The NTP view cannot use the safeAreaInsets property directly as it is
snapshotted before being inserted in the view hierarchy, and the safe
area is only available for views in the view hierarchy.
This CL introduces a property mirroring the safe area once the view is
in the view hierarchy. But it is also set up with the value used in the
first frame, allowing the snapshot to be done as if the view was in the
view hierarchy.

Reland of https://chromium-review.googlesource.com/c/chromium/src/+/681942
It was reverted because the NTP wasn't working correctly when opening
the recent tabs from the tool menu.

Bug:  767869 
Change-Id: Ibf57b84dd6aa4447101c0d0828e5f9af3d76670d
Reviewed-on: https://chromium-review.googlesource.com/691727
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505349}
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_bar.h
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_bar.mm
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_controller.h
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_controller.mm
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_view.h
[modify] https://crrev.com/d889045ed5be75ca9eb5301459ce29dc03df0e69/ios/chrome/browser/ui/ntp/new_tab_page_view.mm

Comment 6 by srikanthg@chromium.org, Oct 10 2017

Status: Verified (was: Fixed)
Bottom bar looks good.
Verified on M63.0.3237.0 canary
Device: iPhone X Sim
https://drive.google.com/file/d/0B-xmXLQhjeKucFdmaTFDa2xLdTg/view
►

Sign in to add a comment