NewTab animation is not smooth while a phone call is in progress. |
|||||||||||||||||||
Issue descriptionApp Version: 62.0.3187.0 canary iOS Version: 11.0, 10.3.3 Device: iPhone7 plus, iPhone6s URL: NTP Steps to reproduce: [From: justincohen@] 1. Make a phone call from the device 2. Background the call in progress 3. Cold start Google Chrome canary 4. Open a new tab Observed results: Observe that NewTab animation is not smooth. Bookmarks and RecentTabs icons jumps a little bit while opening up new tab. Also note tapping on tools menu is also not correct. Expected results: NewTab animation should be correct. Number of times you were able to reproduce: 5/5 Bug reproducible after clean install: Yes Bug reproducible after clearing cache and cookies: Yes Bug reproducible on Chrome Mobile on Android: NA Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA Bug reproducible on current stable build (App Version, iOS Version): M60 Yes Bug reproducible on the current beta channel build (App Version, iOS Version): M61 Yes Link to video/image: https://drive.google.com/file/d/0B-xmXLQhjeKuQkdiQ2R4WDkwTDQ/view
,
Aug 16 2017
,
Aug 29 2017
I think it's more complicated than that. I think the -StatusBarHeight hack in ui_util.mm is no longer valid on iOS11, and a lot of the assumptions made about what happens during a phone call are therefor wrong.
I don't know what the right solution is, in that case.
For example, this 'fixes' the problem, but I don't know what the real solution is.
kkhorimoto@ can you lend a hand here, I think you know more about BVC layout solutions, perhaps?
diff --git a/ios/chrome/browser/ui/browser_view_controller.mm b/ios/chrome/browser/ui/browser_view_controller.mm
index 183de5d9c4a4..5ed67ce2e22c 100644
--- a/ios/chrome/browser/ui/browser_view_controller.mm
+++ b/ios/chrome/browser/ui/browser_view_controller.mm
@@ -2303,10 +2303,7 @@ bubblePresenterForFeature:(const base::Feature&)feature
}
- (UIImageView*)pageFullScreenOpenCloseAnimationView {
- CGRect viewBounds, remainder;
- CGRectDivide(self.view.bounds, &remainder, &viewBounds, StatusBarHeight(),
- CGRectMinYEdge);
- return [[UIImageView alloc] initWithFrame:viewBounds];
+ return [[UIImageView alloc] initWithFrame:self.contentArea.frame];
}
- (UIImageView*)pageOpenCloseAnimationView {
diff --git a/ios/chrome/browser/ui/ui_util.mm b/ios/chrome/browser/ui/ui_util.mm
index 527abe026097..8b3f523292fb 100644
--- a/ios/chrome/browser/ui/ui_util.mm
+++ b/ios/chrome/browser/ui/ui_util.mm
@@ -56,7 +56,7 @@ CGFloat StatusBarHeight() {
// Checking [UIApplication sharedApplication].statusBarFrame will return the
// wrong offset when the application is started while in a phone call, so
// simply return 20 here.
- return 20;
+ return [UIApplication sharedApplication].statusBarFrame.size.height;
}
CGFloat AlignValueToPixel(CGFloat value) {
,
Aug 29 2017
I'm unfamiliar with this code; the only BVC layout stuff I've worked with is the header height/native controller stuff. This seems to be specific to UIKit behavior, and I don't know enough about the new tab animations to tell if changing StatusBarHeight() is the right choice here.
,
Sep 4 2017
,
Sep 6 2017
justincohen@ please what's the status on this blocker? We aim to fix our blockers as early in the cycle as possible.
,
Sep 12 2017
I think the fix for this is going to be very similar to the fix needed for iPhone X status bar handling. Flipping to kkhorimoto since this won't be something I can realistically fix before going on leave.
,
Sep 12 2017
Passing this along to Rohit since I'm working on Bijoi in Paris next week and will be prepping for most of this week. I can take a look when I get back from Paris if this is still a problem, but again, I'm not familiar with this code.
,
Sep 12 2017
From: https://www.hackingwithswift.com/articles/12/how-to-update-your-app-design-for-iphone-x "The status bar is now no longer fixed at 20 points in height, so double check you haven’t hard-coded that figure anywhere. If your app hides the status bar to reclaim space, Apple are asking you to politely reconsider your choice because let’s face it: there wasn’t much you could put up in that notch anyway." Also note: prefersHomeIndicatorAutoHidden
,
Sep 12 2017
,
Sep 14 2017
rohitrao@ are you working on this issue? Please update the bug accordingly
,
Sep 14 2017
Peter, this is closely related to all of the iPhoneX statusbar bugs, so it should go to whoever is going to work on those.
,
Sep 14 2017
,
Sep 15 2017
The bug is not specific to iPhone X (original report about iPhone 6/7) and exists in earlier version (at least M60/61). I don't think the root cause is clear, so it's OK to keep the Hotlist-iPhoneX label for now. However, the priority may need to be re-evaluated.
,
Sep 15 2017
I think I have a fix for this specific bug. The iPhoneX side of this bug has a much larger scope.
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13b7c43252e0c2590ef92e0a27eaa186c7f6b235 commit 13b7c43252e0c2590ef92e0a27eaa186c7f6b235 Author: Justin Cohen <justincohen@google.com> Date: Fri Sep 15 12:40:09 2017 [ios] Fix layout issues when starting phone in voice call. UIKit seems to be insetting our root view controller by 20 points when in a phone call, but setting our main_view_controller to self.window.bounds is stomping on that. It's seems that pre-iOS11 this inset would still happen later. This issue appears when compiling against the iOS11 SDK, even when running on iOS10 or iOS9 devices, it seems. Bug: 755974 Change-Id: I25b6e3848ae506b5531b5eb28348ac5328a6c779 Reviewed-on: https://chromium-review.googlesource.com/668046 Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org> Reviewed-by: Mark Cogan <marq@chromium.org> Commit-Queue: Justin Cohen <justincohen@chromium.org> Cr-Commit-Position: refs/heads/master@{#502230} [modify] https://crrev.com/13b7c43252e0c2590ef92e0a27eaa186c7f6b235/ios/chrome/browser/ui/browser_view_controller.mm [modify] https://crrev.com/13b7c43252e0c2590ef92e0a27eaa186c7f6b235/ios/chrome/browser/ui/main/main_coordinator.mm
,
Sep 15 2017
I landed a fix unrelated to iPhone X, although later fixes will come for that and are related to crrev/c/668046, this issue is now resolved.
,
Sep 15 2017
[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.
,
Sep 15 2017
,
Sep 15 2017
This bug requires manual review: Less than 28 days to go before AppStore submit on M62 Please contact the 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
,
Sep 19 2017
Verified on M63.0.3219.0 canary, dev Device: iPhone7 plus iOS: 11 GM seed. New tab animation looks good while call is in progress.
,
Sep 19 2017
,
Sep 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65537ac4515483363116fa484dfaee95607bb36f commit 65537ac4515483363116fa484dfaee95607bb36f Author: Justin Cohen <justincohen@google.com> Date: Tue Sep 19 18:03:46 2017 [ios] Fix layout issues when starting phone in voice call. UIKit seems to be insetting our root view controller by 20 points when in a phone call, but setting our main_view_controller to self.window.bounds is stomping on that. It's seems that pre-iOS11 this inset would still happen later. This issue appears when compiling against the iOS11 SDK, even when running on iOS10 or iOS9 devices, it seems. TBR=justincohen@google.com (cherry picked from commit 13b7c43252e0c2590ef92e0a27eaa186c7f6b235) Bug: 755974 Change-Id: I25b6e3848ae506b5531b5eb28348ac5328a6c779 Reviewed-on: https://chromium-review.googlesource.com/668046 Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org> Reviewed-by: Mark Cogan <marq@chromium.org> Commit-Queue: Justin Cohen <justincohen@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#502230} Reviewed-on: https://chromium-review.googlesource.com/673423 Reviewed-by: Justin Cohen <justincohen@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#324} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/65537ac4515483363116fa484dfaee95607bb36f/ios/chrome/browser/ui/browser_view_controller.mm [modify] https://crrev.com/65537ac4515483363116fa484dfaee95607bb36f/ios/chrome/browser/ui/main/main_coordinator.mm
,
Sep 27 2017
Verified the issue on 62.0.3202.35 beta tested on iPhone7+(11.0.1) New tab animation looks good while call is in progress. |
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by gambard@chromium.org
, Aug 16 2017