Part of the PDF file hidden behind the top toolbar when opened from web page |
||||||
Issue descriptionApp Version: 71.0.3578.45 Beta iOS Version: 11.4.1 only Device: iPhone and 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 2. Search for sample pdf file and tap on the pdf link (or Open irs.gov --> Browse to a PDF file) Observed results: Notice that top of the pdf file overlaps with the top toolbar Expected results: Webview should display pdf file from the top 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: NA, Safari: NA Bug reproducible on current stable build (App Version, iOS Version): NA on M70 (Feature available from M71) Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M71 Bet Link to Video: https://drive.google.com/file/d/1m5z6bGJCqjiypSOB4XNwoHVuig-YcDz9/view?usp=sharing
,
Nov 9
,
Dec 11
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/85d468063100a8839483270aad4a7845fc705b3a commit 85d468063100a8839483270aad4a7845fc705b3a Author: Kurt Horimoto <kkhorimoto@chromium.org> Date: Wed Dec 12 19:28:26 2018 [iOS] Move pdfs below headers when first loaded on iOS 11. On iOS 12, resetting the WKScrollView.contentInset at the start of the navigation pushes the content to {0, -contentInset.top}. On iOS 11, this does not occur. This CL updates FullscreenWebStateObserver to push the content below the toolbar the firs time a NavigationItem is loaded with the shouldUseViewContentInset property set to YES. This CL also updates MoveContentHelowHeader() to use the current top toolbar inset rather than progress * extended toolbar height. The previous value was only working correctly because the function was only called with a progress value of 1.0. This CL re-enables FullscreenTestCase.testLongPDFInitialState, as the behavior being tested is now fixed. Bug: 904694 , 903247 Change-Id: I134461e16054846a211a5476dd5a6694bd709002 Reviewed-on: https://chromium-review.googlesource.com/c/1371082 Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Cr-Commit-Position: refs/heads/master@{#615997} [modify] https://crrev.com/85d468063100a8839483270aad4a7845fc705b3a/ios/chrome/browser/ui/fullscreen/fullscreen_content_adjustment_util.mm [modify] https://crrev.com/85d468063100a8839483270aad4a7845fc705b3a/ios/chrome/browser/ui/fullscreen/fullscreen_egtest.mm [modify] https://crrev.com/85d468063100a8839483270aad4a7845fc705b3a/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_list_observer_unittest.mm [modify] https://crrev.com/85d468063100a8839483270aad4a7845fc705b3a/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_observer.mm
,
Dec 12
,
Dec 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed4acaf3500449e78001fff77e6062ecadd6255c commit ed4acaf3500449e78001fff77e6062ecadd6255c Author: Kurt Horimoto <kkhorimoto@chromium.org> Date: Mon Dec 17 21:37:53 2018 [iOS] Move pdfs below headers when first loaded on iOS 11. On iOS 12, resetting the WKScrollView.contentInset at the start of the navigation pushes the content to {0, -contentInset.top}. On iOS 11, this does not occur. This CL updates FullscreenWebStateObserver to push the content below the toolbar the firs time a NavigationItem is loaded with the shouldUseViewContentInset property set to YES. This CL also updates MoveContentHelowHeader() to use the current top toolbar inset rather than progress * extended toolbar height. The previous value was only working correctly because the function was only called with a progress value of 1.0. This CL re-enables FullscreenTestCase.testLongPDFInitialState, as the behavior being tested is now fixed. Bug: 904694 , 903247 Change-Id: I134461e16054846a211a5476dd5a6694bd709002 Reviewed-on: https://chromium-review.googlesource.com/c/1371082 Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#615997}(cherry picked from commit 85d468063100a8839483270aad4a7845fc705b3a) Reviewed-on: https://chromium-review.googlesource.com/c/1381132 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#407} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/ed4acaf3500449e78001fff77e6062ecadd6255c/ios/chrome/browser/ui/fullscreen/fullscreen_content_adjustment_util.mm [modify] https://crrev.com/ed4acaf3500449e78001fff77e6062ecadd6255c/ios/chrome/browser/ui/fullscreen/fullscreen_egtest.mm [modify] https://crrev.com/ed4acaf3500449e78001fff77e6062ecadd6255c/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_list_observer_unittest.mm [modify] https://crrev.com/ed4acaf3500449e78001fff77e6062ecadd6255c/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_observer.mm
,
Dec 18
Verified on 73.0.3644.0 Canary, iPhoneXR iOS 12.1.1, iPad Pro iOS12.1.1, iPhone X iOS11.4.1. Looks good
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed4acaf3500449e78001fff77e6062ecadd6255c Commit: ed4acaf3500449e78001fff77e6062ecadd6255c Author: kkhorimoto@chromium.org Commiter: kkhorimoto@chromium.org Date: 2018-12-17 21:37:53 +0000 UTC [iOS] Move pdfs below headers when first loaded on iOS 11. On iOS 12, resetting the WKScrollView.contentInset at the start of the navigation pushes the content to {0, -contentInset.top}. On iOS 11, this does not occur. This CL updates FullscreenWebStateObserver to push the content below the toolbar the firs time a NavigationItem is loaded with the shouldUseViewContentInset property set to YES. This CL also updates MoveContentHelowHeader() to use the current top toolbar inset rather than progress * extended toolbar height. The previous value was only working correctly because the function was only called with a progress value of 1.0. This CL re-enables FullscreenTestCase.testLongPDFInitialState, as the behavior being tested is now fixed. Bug: 904694 , 903247 Change-Id: I134461e16054846a211a5476dd5a6694bd709002 Reviewed-on: https://chromium-review.googlesource.com/c/1371082 Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#615997}(cherry picked from commit 85d468063100a8839483270aad4a7845fc705b3a) Reviewed-on: https://chromium-review.googlesource.com/c/1381132 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#407} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 19
Issue verified Version: Chrome Beta 72.0.3626.28 Device: iPad Mini iOS: 11.4.1 No overlapping https://drive.google.com/open?id=1Q8pcCdYoyKDIDQR-HEAtMUr9fy9-v651 |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by kkhorimoto@chromium.org
, Nov 8Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)