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

Issue 778229 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Enable by default the Safe Area Compatible Toolbar flag

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

Issue description

ios/chrome/browser/ui/toolbar/toolbar_controller_base_feature.mm
 

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

Enabling the "Safe Area Compatible Toolbar" flag seems to break some tests:
https://chromium-review.googlesource.com/c/chromium/src/+/738182

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

Cc: sczs@chromium.org rohitrao@chromium.org

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

Components: UI>Browser>Toolbar

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

Status: Available (was: Started)
Labels: -ReleaseBlock-Stable -M-63
Removing the M63 RBS label as the flag won't be enabled on M63.
Project Member

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

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

commit a7d8e4a071d58e068579352a85f235756a691a8c
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Nov 06 14:58:17 2017

Enable Safe Area compatible toolbar on iPhone X

This CL enables the Safe Area compatible toolbar flag for iPhone X by
default.
In order to have the flag only enabled for iPhone X, it moves the
feature declaration to a protected file and add an utility function.

Bug:  778229 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id01c2237c50e7848027611a8503910b0aff5bf82
Reviewed-on: https://chromium-review.googlesource.com/754603
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514129}
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/stack_view/stack_view_controller.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/public/BUILD.gn
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/public/toolbar_utils.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/toolbar_controller.mm
[add] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/toolbar_private_base_feature.h
[add] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/toolbar_private_base_feature.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/toolbar_view.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/ui_util.h
[modify] https://crrev.com/a7d8e4a071d58e068579352a85f235756a691a8c/ios/chrome/browser/ui/ui_util.mm

Project Member

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

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

commit 21670fda4bdc2454f3c3f31dc7606eba0a17e2b8
Author: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Date: Mon Nov 06 20:07:58 2017

Revert "Enable Safe Area compatible toolbar on iPhone X"

This reverts commit a7d8e4a071d58e068579352a85f235756a691a8c.

Reason for revert: 

<unknown>:0: error: -[ReadingListTestCase testSavingToReadingListAndLoadBadNetwork] : failed: caught "NSGenericException", "Unable to activate constraint with anchors <NSLayoutXAxisAnchor:0x604000a62f80 "kToolbarIdentifier.leading" (names: kToolbarIdentifier:0x7fbaa8d33cc0)> and <NSLayoutXAxisAnchor:0x604001c78a00 "UIView:0x7fbaa8e29930.leading"> because they have no common ancestor.  Does the constraint or its anchors reference items in different view hierarchies?  That's illegal."
(
	0   CoreFoundation                      0x00000001146181cb __exceptionPreprocess + 171
	1   libobjc.A.dylib                     0x0000000118edbf41 objc_exception_throw + 48
	2   CoreFoundation                      0x000000011468cb95 +[NSException raise:format:] + 197
	3   ios_chrome_reading_list_egtests     0x000000010804bd8d -[StackViewController animateTransitionToolbarWithCardFrame:transitionStyle:] + 1197
	4   ios_chrome_reading_list_egtests     0x0000000108046b57 -[StackViewController animateTransitionWithStyle:] + 3799
	5   ios_chrome_reading_list_egtests     0x0000000108043f70 -[StackViewController showWithSelectedTabAnimation] + 48
	6   ios_chrome_reading_list_egtests     0x00000001080ce302 -[MainContainingViewController showTabSwitcher:completion:] + 114

Original change's description:
> Enable Safe Area compatible toolbar on iPhone X
> 
> This CL enables the Safe Area compatible toolbar flag for iPhone X by
> default.
> In order to have the flag only enabled for iPhone X, it moves the
> feature declaration to a protected file and add an utility function.
> 
> Bug:  778229 
> Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: Id01c2237c50e7848027611a8503910b0aff5bf82
> Reviewed-on: https://chromium-review.googlesource.com/754603
> Reviewed-by: Mark Cogan <marq@chromium.org>
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#514129}

TBR=marq@chromium.org,gambard@chromium.org

Change-Id: Iadcc40735ed123259b8a45e8ab9c4d5b97efa8f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  778229 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/755574
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Commit-Queue: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514221}
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/stack_view/stack_view_controller.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/public/BUILD.gn
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/public/toolbar_utils.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/toolbar_controller.mm
[delete] https://crrev.com/863f513de728c76b7bb0406f0cb1f2a08f8960fc/ios/chrome/browser/ui/toolbar/toolbar_private_base_feature.h
[delete] https://crrev.com/863f513de728c76b7bb0406f0cb1f2a08f8960fc/ios/chrome/browser/ui/toolbar/toolbar_private_base_feature.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/toolbar_view.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/ui_util.h
[modify] https://crrev.com/21670fda4bdc2454f3c3f31dc7606eba0a17e2b8/ios/chrome/browser/ui/ui_util.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2017

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

commit ba27610e4f6c4a2b4f02327e265f1b2a19145a0a
Author: Justin Cohen <justincohen@google.com>
Date: Wed Nov 08 19:34:45 2017

Reland "Enable Safe Area compatible toolbar on iPhone X"

This CL enables the Safe Area compatible toolbar flag for iPhone X by
default.
In order to have the flag only enabled for iPhone X, it moves the
feature declaration to a protected file and add an utility function.

This reverts commit 21670fda4bdc2454f3c3f31dc7606eba0a17e2b8.

Bug:  778229 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I1ba581f1238a935b47bce2171d553ca759dbad5b
Reviewed-on: https://chromium-review.googlesource.com/757582
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514906}
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/ntp/new_tab_page_header_view.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/stack_view/stack_view_controller.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/public/BUILD.gn
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.h
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/public/toolbar_controller_base_feature.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/public/toolbar_utils.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/toolbar_controller.mm
[add] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/toolbar_private_base_feature.h
[add] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/toolbar_private_base_feature.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/toolbar_view.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/ui_util.h
[modify] https://crrev.com/ba27610e4f6c4a2b4f02327e265f1b2a19145a0a/ios/chrome/browser/ui/ui_util.mm

Cc: jif@chromium.org
Owner: justincohen@chromium.org
Status: Fixed (was: Available)
Status: Verified (was: Fixed)
Enable SafeArea Compatible Toolbar flag is enabled by default on M64 with iPhoneX
Verified on M64.0.3267.0 canary
Device: iPhoneX iOS:11.2
Issue 788538 has been merged into this issue.

Sign in to add a comment