New issue
Advanced search Search tips

Issue 763899 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

automaticallyAdjustsScrollViewInsets is deprecated with iOS11.

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

Issue description

On iOS11, Chrome iOS' UICollectionViews (Settings, Reading List, History...) currently have an unwanted 20 point inset.

Fix may be to use the new iOS11 |contentInsetAdjustment| property.
 

Comment 1 by jif@chromium.org, Sep 11 2017

Note that this is impacting https://bugs.chromium.org/p/chromium/issues/detail?id=763602
Cc: linds...@chromium.org mard...@chromium.org
Labels: Hotlist-iOS11
I have a fix for all collection, settings the property introduced in iOS 11 to what should be value corresponding to the iOS 10 behavior.
However, this might be breaking a collection in an unexpected way. I have tested 5-6 collections, they all had the problem and they all seem fine.

Should we cherry-pick it for all collection?
I think there are more chances to have something broken because this fix is not cherry-picked rather than with it cherry-picked.
It should not be introducing a crash, worse case scenario there is a collection overlapping the status bar or with few pixels hidden.

I have attached two screenshots to see the difference.
Simulator Screen Shot - iPhone 6s - 2017-09-11 at 15.59.43.png
136 KB View Download
Simulator Screen Shot - iPhone 6s - 2017-09-11 at 16.02.11.png
136 KB View Download
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 11 2017

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

commit 07949c53ea29e8b282e71ef63b48b453a4b98c45
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Sep 11 14:22:19 2017

Fix top collection padding

iOS 11 introduce contentInsetAdjustmentBehavior, which creates a 20pt
spacing at top of the collections. This property has been introduce to
have an easy way to handle the status bar and its height.
This CL sets it to UIScrollViewContentInsetAdjustmentNever to remove
the spacing. This value seems to be equivalent to the pre-iOS 11
behavior.

This CL affects all the CollectionViewController.

Bug:  763899 
Test: Open the collections through the app and make sure that there is no longer a 20pt margin at the top. Also make sure that no collection is overlapping the status bar.
Change-Id: Id1163ff3f723800c2fce7f8947b17507af0789ef
Reviewed-on: https://chromium-review.googlesource.com/660257
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500908}
[modify] https://crrev.com/07949c53ea29e8b282e71ef63b48b453a4b98c45/ios/chrome/browser/ui/collection_view/collection_view_controller.mm

Status: Fixed (was: Assigned)
Fixed on M63.
To verify it, you can see the difference on the screenshots in #2.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-62; 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-62 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-62
Asking for merge.
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 13 2017

Labels: -Merge-Request-62 Hotlist-Merge-Approved Merge-Approved-62
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Approved -ReleaseBlock-Stable -Merge-Approved-62
I don't think it is a good idea to cherry-pick this with the safe area used in iPhone X.
Retracting this merge, a global solution should be found.
Status: Verified (was: Fixed)
Verified on iOS 11 iPhone 6+ on build 63.0.3216.0 Canary.
https://drive.google.com/a/google.com/file/d/0B6GVWQnhaMClMkpOV0VQZzY2Wmc/view?usp=sharing

Comment 10 by pkl@chromium.org, Sep 19 2017

Labels: Hotlist-iPhoneX

Comment 11 by pkl@chromium.org, Oct 2 2017

Cc: gambard@chromium.org
 Issue 770302  has been merged into this issue.

Sign in to add a comment