New issue
Advanced search Search tips

Issue 896274 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

PDF open in button is no displayed above the bottom toolbar

Project Member Reported by rakurati@chromium.org, Oct 17

Issue description


App Version: 71.0.3578.9 Beta
iOS Version: 12.0, 11.4.1, 10.3.3
Device: iPhone, iPad

Prerequisite: 
1. Enable ‘#out-of-web-fullscreen’ flag from chrome://flags
2. Enable ‘#browser-container-fullscreen’ flag from chrome://flags
 
Steps to reproduce:
1. Launch chrome app and set the above mentioned flags
2. Cold start the app
3. Load any sample pdf file (http://www.pdf995.com/samples/pdf.pdf)
4. When both top and bottom toolbar is displayed, tap anywhere on the pdf file

Note: For iPad’s after step 3, scroll the pdf file slightly and tap on the pdf file

Observed results:
Notice that a open in button is not displayed above the toolbar

Expected results:
Open in button should be displayed above the toolbar

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: Not tested
Bug reproducible on Safari/Firefox: Firefox: No, Safari: No
Bug reproducible on current stable build (App Version, iOS Version): NA on M69 
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M71 Beta (Feature available from M71)

Link to video:
https://drive.google.com/file/d/1LypLCCormadjIEcsapHmBEEo_ka0elCe/view?usp=sharing

 
Cc: gambard@chromium.org
Labels: -Pri-2 ReleaseBlock-Stable M-71 Pri-1
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
Cc: kkhorimoto@chromium.org
Owner: gambard@chromium.org
Labels: Needs-Feedback
Should be fixed with the fix in issue 891634. Could you check again (today's Canary is fine)?
Labels: -Needs-Feedback
Verified in 72.0.3585.0 Canary in iPhone 8plus(iOS 12 beta 4), iPhone 8(iOS 11.4.1), iPad Air(iOS 12), iPad 2018(iOS 11.4.1)

In chrome://flags enable:
    - Browser Container Fullscreen
    - Fullscreen implementation out of web
    - "Fullscreen Viewport Adjustment Mode" to "Smooth Scrolling"

Not able to reproduce the above issue, looks good

Link to video:
https://drive.google.com/file/d/18laPvDk77ol52RBquQAVBeGWEH0FIpSs/view?usp=sharing
Status: Fixed (was: Assigned)
Thanks, closing this.
Please verify in next M71, the fix was merged today.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; 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-71 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD
Status: Assigned (was: Fixed)
Tested in 71.0.3578.23 beta in iPhone 8(iOS 11.4.1), iPhone X(iOS 11.4.1), iPad 2018(iOS 11.4.1), iPhone 6 plus(iOS 10.3.3), iPhone 6S plus(iOS 12 beta5)

In chrome://flags enable:
    - Browser Container Fullscreen
    - Fullscreen implementation out of web
    - "Fullscreen Viewport Adjustment Mode" to "Smooth Scrolling"

Able to reproduce the above issue in iOS 10 and 11. Looks good in iOS 12

Note: Works fine on all versions in M72 Canary

Link to video:
iOS 11 Behavior:
https://drive.google.com/file/d/1FHwAm3vE8EcNSzLUupdyhoJY_0MKmR0d/view?usp=sharing

iOS 12 Behavior:
https://drive.google.com/file/d/16PnGjDD7nimOXglGLzS2sfhW8s4Zm7F4/view?usp=sharing
Cc: kariahda@chromium.org
Labels: Merge-Request-71
It has been fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1261523.
Asking for merge request as it is already verified (here) in M72.
Thanks for catching it rakurati@!
Status: Fixed (was: Assigned)
Project Member

Comment 11 by sheriffbot@chromium.org, Oct 30

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Canary verification please. How confident are you in this fix? Can it brake something else?
We got Canary verification in #8 (i.e. "Works fine on all versions in M72 Canary").
I don't think it should break something, especially considering it has been in Canary for more than 10 days.
Per c8 this is still broken in iOS 10 and 11 for M71, correct? Are you just planning to fix iOS 12 for M71 and then let iOS 10 and 11 users wait until M72 for the fix to work for them?
Sorry I don't understand. The CL is fixing it for all the iOS version. The CL I linked in #9 hasn't been cherry-picked.
The fix is making sure that the tests are passing but not only by fixing the tests, also by fixing the bugs that the tests revealed.
@kariahda, The CL from comment#9 is already been tested in M72 Canary and the fix looks good.

Since this issue is marked as RBS, we can't sign off the fullscreen feature with out verifying this CL in M71 Beta. Could you please merge the CL from comment#9.
Labels: -Merge-Review-71 Merge-Approved-71
Merge approved. From c8, it seemed to me that maybe this fix was only for iOS 12. Now I see this is not the case.
Status: Verified (was: Fixed)
Verified in 71.0.3578.45 Beta in iPhone 8plus(iOS 11.4.1), iPhone X(iOS 12.0.1), iPad 2018(iOS 11.4.1), iPhone 6 plus(iOS 10.3.3)

Not able to reproduce the above issue, looks good

Link to video:
https://drive.google.com/file/d/1Tj3Dfl3HKPDE_ykhrCmqDzO_-6B53UIV/view?usp=sharing

Sign in to add a comment