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

Issue 765308 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Bookmarks list and edit views overlap with camera and sensor shelf on iPhoneX in landscape mode

Project Member Reported by linds...@chromium.org, Sep 14 2017

Issue description

App Version: 63.0.3215.0
iOS Version: iOS11
Device: iPhoneX Sim
URL: Bookmarks 

Steps to reproduce:
  1. Launch app
  2. Tap the Star on NTP for bookmarks
  3. Turn device to landscape mode

Observed results:
Observe the left side of the bookmarks overlaps with the camera and sensor shelf in landscape mode. On the right side it will overlap the edit options. These overlaps happen in Edit/Move modes as well.

Expected results:
No overlap should occur.

Screenshot: https://drive.google.com/file/d/0By4O1f2IQqQ_SzNWQmJVUC1RRjQ/view
https://drive.google.com/file/d/0By4O1f2IQqQ_SWdWSF9adnJJZGc/view
 

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

Cc: ramyasharma@chromium.org
Owner: stkhapugin@chromium.org
Status: Assigned (was: Untriaged)
Assigning to stk@ because ramyasharma@ is OOO
Cc: martiw@chromium.org

Comment 3 by pkl@chromium.org, Sep 20 2017

Labels: ReleaseBlock-Stable M-63
This impacts feature usability. Setting to M63 RBS.
Status: Started (was: Assigned)
Project Member

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

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

commit 6b701428aa2a98d01de2ec1fd27a9227ca5684eb
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Thu Sep 21 17:00:29 2017

Fix bookmarks on iPhone X.

Fixes toolbars (standard and edit) and bookmark list to respect the
safe areas. The fixes are not great, but seem to be acceptable since
bookmarks will soon be replaced with new bookmarks.

Bug:  765308 
Change-Id: I5c9fd374eee251cfca5ddecdc71ea4cbecb5b86e
Reviewed-on: https://chromium-review.googlesource.com/677384
Reviewed-by: Eric Noyau <noyau@chromium.org>
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503477}
[modify] https://crrev.com/6b701428aa2a98d01de2ec1fd27a9227ca5684eb/ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.mm
[modify] https://crrev.com/6b701428aa2a98d01de2ec1fd27a9227ca5684eb/ios/chrome/browser/ui/bookmarks/bars/bookmark_top_bar.mm
[modify] https://crrev.com/6b701428aa2a98d01de2ec1fd27a9227ca5684eb/ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.mm

Status: Fixed (was: Started)
Cc: linds...@chromium.org
Status: Verified (was: Fixed)
Toolbars looking good. Bookmarks list is respecting the safe areas now.
Verified on iPhoneX Simulator M64.0.3251.0 canary, M63.0.3239.19 beta

Bookmarks Edit UI Screen still doesn't respect safearea.(Screenshot#2 from comment#0) With old Bookmarks UI and New Gen Bookmarks UI.

Let me know if we need to file a new bug for that.
@stk was your fix supposed to address the list and edit views of Bookmarks?
Ping @stk?
Status: Assigned (was: Verified)
I'm sorry, I completely missed the list aspect, and since this was verified, I never saw this bug again. I'll take a look 

Status: Started (was: Assigned)
The _edit_ aspect, not the list aspect, sorry for possible confusion.
Cc: pkl@chromium.org
Status: Assigned (was: Started)
So I looked at it, and this is fine on trunk, but broken on 63. The fix is in MDC, so I see three options:

1. Do nothing, because this is only broken in Landscape on iPhone X, and it's not unusable. Looks totally fine in Portrait. 
2. Cherry-pick an MDC roll and possibly a piper roll into M63.
3. Write a small cherry-pickable fix specifically for M63 and let the branch diverge from trunk. 

I think 2 is very risky and hard. 3 is pretty ugly and it's not a great idea to diverge M63, since this is not something we generally do. So I prefer to keep it as is; especially given that landscape usage on phones is low and bookmarks usage is low, too.

+pkl, you were the main POC for the piper/MDC rolls lately, could you say how you feel about option 2?
Cc: stkhapugin@chromium.org
Owner: pkl@chromium.org
Assigning to pkl@ for feedback. Please return to me :)

Comment 14 by pkl@chromium.org, Nov 10 2017

Labels: -ReleaseBlock-Stable
Owner: stkhapugin@chromium.org
For M63, we will launch Bookmarks NG which does not have the same problem to 100%.
I think the answer is actually "WontFix". Reassigning back to stk@ to actually WontFix this bug.

Removing ReleaseBlock-Stable at this time.
Status: WontFix (was: Assigned)

Sign in to add a comment