New issue
Advanced search Search tips

Issue 834131 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug

Blocking:
issue 835046



Sign in to add a comment

Fullscreen is constantly reset on pitchfork.com

Project Member Reported by kkhorimoto@chromium.org, Apr 18 2018

Issue description

When scrolling on pitchfork.com, the fullscreen progress is continuously reset to 1.0 (i.e. fully visible toolbar).  This is occurring because the page is continuously performing window.location.replaceState() calls to update the URL.  This is going through FullscreenWebStateObserver::DidFinishNavigation(), which resets the progress to 1.0 to show the toolbar.

It's unclear what the correct fix for this would be.  The NavigationContext passed to the WSO callback contains whether the navigation is same-document, but I think we'd still want to show the toolbar when the document changes for pushState() navigations, so we wouldn't be able to use that as a signal for whether to reset the model.  The web// layer tracks whether a navigation is due to a replaceState() call, but this information is currently private, and would need to be exposed in the public interface in order to differentiate these navigations.  This would be a departure from content//'s public interface.
 
Do we know how Chrome for Android behaves? Personally, I don't think we should exit fullscreen for the same-document navigations. The URL origin does not change, and omnibox (at least with UI refresh flag enabled) only displays the origin.
Status: Started (was: Assigned)
It looks like clank doesn't reset fullscreen for same-document navigations, so I've added that functionality here: crrev.com/c/https://chromium-review.googlesource.com/c/chromium/src/+/1017231
Sorry, I messed up the link.  The CL is here: crrev.com/c/1017231
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 19 2018

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

commit 0e1921e809e26731c9a9b037e7301068c096e212
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Thu Apr 19 21:29:25 2018

[iOS] Don't reset fullscreen state for same-document navigations.

Bug:  834131 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7a3bb44da51bdd3aac19ff8da559d4178e0fe168
Reviewed-on: https://chromium-review.googlesource.com/1017231
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552164}
[modify] https://crrev.com/0e1921e809e26731c9a9b037e7301068c096e212/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_observer.mm
[modify] https://crrev.com/0e1921e809e26731c9a9b037e7301068c096e212/ios/chrome/browser/ui/fullscreen/fullscreen_web_state_observer_unittest.mm

Status: Fixed (was: Started)
Labels: MS-Fullscreen S-Fullscreen-State-Operations
Blocking: 835046

Sign in to add a comment