Content Suggestion article content os covered by Toolbar |
|||||||||
Issue descriptionApp Version (from "Chrome Settings > About Chrome"): 63.0.3221.0 dev iOS Version: iOS 11 Device: iPhone SE Steps to reproduce: 1.) Open NTP 2.) Scroll to the bottom to reveal content suggestions article 3.) Long press on the article 4.) Tap "Open in New Tab" 5.) Switch to the newly opened Tab by side-swiping the toolbar Observed behavior: Top part of the page content is covered by the toolbar permanently. Expected behavior: Page content should not be hidden behind the toolbar. Additional comments: Does not reproduce on every web site. Reproducible on kotaku.com f.e.
,
Sep 28 2017
The bug is probably somewhere in Fullscreen code.
,
Sep 28 2017
,
Oct 10 2017
Hi, This is listed as a P1 for M62. Are there any updates for this? Thanks,
,
Oct 10 2017
Yeah this looks like a fullscreen issue. I'll take a look at it today or tomorrow.
,
Feb 21 2018
This issue is still reproduced as of M66.0.3351.0 canary Tested on iPhone7 plus, 10.3.3 iPhoneX iOS11.3.
,
Feb 21 2018
,
Feb 23 2018
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6 commit e2ec670f6f93cf2994334587b4ab03aa0a9bdca6 Author: Kurt Horimoto <kkhorimoto@chromium.org> Date: Fri Feb 23 22:33:55 2018 [iOS] Move content below header the first time a WebState is activated. This CL also renames UpdateFullscreenWebViewProxyForReplacedScrollView() to MoveContentBelowHeader(), as it is used in additional places other than just for replaced scroll views. Bug: 769379 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I330e20e33501429140a8ea3dfd9e8afa34d47fcf Reviewed-on: https://chromium-review.googlesource.com/933921 Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: Justin Cohen <justincohen@chromium.org> Cr-Commit-Position: refs/heads/master@{#538925} [modify] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/BUILD.gn [add] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/fullscreen_content_adjustment_util.h [rename] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/fullscreen_content_adjustment_util.mm [modify] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_list_observer.h [modify] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_list_observer.mm [modify] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_list_observer_unittest.mm [modify] https://crrev.com/e2ec670f6f93cf2994334587b4ab03aa0a9bdca6/ios/chrome/browser/ui/fullscreen/fullscreen_web_view_proxy_observer.mm [delete] https://crrev.com/9f37416d6fef266dcaeed0b09d0c79584a3d71ef/ios/chrome/browser/ui/fullscreen/fullscreen_web_view_scroll_view_replacement_util.h
,
Feb 23 2018
+cmasso for merge decision. This only reproduces when using the toolbar side swipe gesture to switch tabs, so it might be enough of an edge case that we would not want to merge this to M65.
,
Feb 23 2018
This bug requires manual review: Less than 7 days to go before AppStore submit on M65 Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 26 2018
Is the fix risky?
,
Feb 26 2018
No, I think this is pretty low risk. The CL is mostly a function rename. The new logic just moved page content below the header when a tab is activated for the first time, but the implementation of MoveContentBelowHeader() uses the current fullscreen progress to perform its logic so even if there is a bug and it's called mistakenly, it should never result in a situation where the top of the page's content is out of sync with the toolbar.
,
Feb 26 2018
Since this is not new in M65, let's ship it with M66 instead. Thanks for the fix Kurt!
,
Feb 27 2018
Issue verified Version: Chrome Canary 66.0.3356.0 Device: iPhone SE iOS: 11.2.5 Page content is correctly displayed for content suggestions articles. Not sure how to generate an content suggestions article for kotaku.com, but it works correctly as most visited icon. https://drive.google.com/open?id=1I5FdSoD-efOTFmvxK2aBj-IEzN7xEGUt |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by gambard@chromium.org
, Sep 28 2017Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)