Behavior of window.reload() on presentation receiver page
markafoltz opened this issue · 12 comments
Internally, a developer noted that calling window.location.reload() on a presentation receiver page in Chrome closes the page. (Chrome closes presentations that attempt to navigate.)
The spec isn't explicit about how browsers should behave in this situation.
- Is there a use case for allowing presentations to
reload()? It would be simpler not to allow it, as we don't have steps to restore any PresentationConnections after reload. - If it's not allowed, then this might deserve a small spec update to be explicit about this case.
The main use case I had was being able to iterate on the UI of the page while in development. Normally I'd load the page in a new tab and do it there, but in this case, the receiver pages builds its UI from presentationRequest.send() commands from the main page.
location.reload() seemed like a good way to do that, but I'd be happy with an easier solution.
Got it. The browser could allow the receiver page to reload, but the controller would have to call connection.reconnect() before you could send commands again.
How can we know when the controller becomes able to reconnect to the receiver after 'window.reload()'?
In the case of connecting, a connect event is fired at a PresentationConnection on the controller when the receiver page has been loaded. On the other hand, when the receiver invokes 'window.reload()', a connect event will not be fired because the state of that PresentationConnection will be set to closed and it will not be reconnected automatically.
I agree with @mfoltzgoogle this is application logic. the connection can be closed for any reason and the sender application should handle this.
The developer could add a button to the controller to manually reconnect once the receiver page has finished reloading.
Since this use case is mostly for debugging, I'm not sure it's worth adding a new reload event to the PresentationConnection to signal the controller that a reload had completed and reconnection is possible.
The spec language may need adjustment to note that reloading the receiver should not terminate the presentation and the reloaded presentation retains the same id (to allow reconnect() to work).
Okay, I agree that the major use case is for debugging and a new event for reload wouldn’t seem worth adding.
The proposed resolution here is to update the spec to allow reload() navigations on the receiver page. Since we are not defining the navigation behavior in terms of HTML5, we can continue to "patch" it in our spec.
(This is orthogonal to the problem raised here and could be addressed in a separate issue, but that was prompted by it and touches on the same spec text, so I'll raise it here).
@mfoltzgoogle Are we really not defining navigation behavior in terms of HTML5, though? The Presentation API does not link to the navigate algorithm in HTML, but it still uses the terms defined there, and I'm not sure about the expected behavior in practice.
Typically, the mention of "Chrome closes presentations that attempt to navigate" makes me wonder whether the spec should not be adjusted to clarify the expected behavior when the receiving page attempts to navigate.
The navigate algorithm is described in HTML 5.3. It includes a step that essentially says that if the browsing context is not "allowed to navigate", then the algorithm should be aborted. A SecurityError may be thrown in some cases, and user agents may offer to open the URL in another browsing context otherwise (FWIW, the algorithm is the same in the HTML LS version - see the definition of navigate there - although the description of that step is tad more convoluted).
So, when the Presentation API spec says "The top-level browsing context MUST NOT be allowed to navigate itself", my understanding is that attempts to navigate should simply do nothing (or perhaps throw a SecurityError). I would not expect such attempts to close the browsing context in particular.
I'm not sure whether the exact behavior matters in practice. It could perhaps be left up to implementations, except it's always easier to test a specific behavior than a range of possibilities... In any case, I think the sentence "The top-level browsing context MUST NOT be allowed to navigate itself" should be updated.
If we want to link to the usual navigate algorithm and abort attempts to navigate, then we could simply link to allowed to navigate in the Presentation API, as a way to say that we're patching it to have it return false for receiving browsing context (except for fragment and reload navigations).
If we rather want to close the receiving browsing context when attempts to navigate are made, then we should say so explicitly, as in "The user agent MUST close the receiving browsing context if it attempts to navigate itself, except..."
PS: I mentioned testing, I note we'll need a couple of new test cases in the test suite to check the behavior.
@tidoust That's a good point. The consequences of prohibited navigation should be described more specifically.
By "not defined in terms of HTML5" I believe it's because there's no flag in the HTML5 spec that prohibits a top level browsing context from navigating itself, so we have to add a rule on top of the navigation spec to enforce this.
I feel it is probably more developer friendly to abort the navigation than to close the presentation; there's no privacy downside, and it will be easier for developers to debug that way.
From an implementation point of view somewhat more complicated to do things that way, however.
@tidoust Do you mind filing a separate issue to address writing spec to say what happens on top level navigation?
Closed by PR #454.