Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify rule of navigating away from the navigation scope #646

Closed
mgiuca opened this issue Jan 30, 2018 · 18 comments
Closed

Clarify rule of navigating away from the navigation scope #646

mgiuca opened this issue Jan 30, 2018 · 18 comments

Comments

@mgiuca
Copy link
Collaborator

mgiuca commented Jan 30, 2018

Briefly discussed in #550 but spinning out into its own issue / PR.

@marcoscaceres @kenchris I think we want to rework the rule about navigating away from the navigation scope (as part of my work in #550 to remove the concept of "unbounded" scope).

There are currently two major cases: bounded scope (explicit scope in manifest) and unbounded scope (no scope in manifest). In the bounded case, you aren't allowed to navigate outside of scope at all, except the user agent is allowed to spawn a new top-level browsing context. In the unbounded case, you are allowed to navigate outside of the origin, but there is a recommendation that the UA show the location bar when off-origin (or just open a browser tab).

It makes no sense for these cases to be treated differently (especially as we move away from unbounded scope). But I think the right place to be is somewhere in the middle.

  • It isn't clear to me what the practical UI implications of "a new top-level browsing context" means.
  • A basic reading of the text suggests that if you have bounded scope, off-scope navigations must either fail, or bounce the user out to a new browser tab. Whereas, if you have an unbounded scope, in the event of an off-origin navigation, the UA may either:
    • a) Continue navigating within the app window, or
    • b) Stay within the app window but show some UI indicating that you're off-origin, along with some location bar, or
    • c) Bounce the user out to a new browser tab.

I think, regardless of bounded vs unbounded, (a) should be banned (because it is insecure, as the user will effectively be browsing the web with no knowledge of where they are), and (b) and (c) should be allowed. On Chrome for Android, we do (b) for both bounded and unbounded scope. Are we violating the spec? Or can we justify it in that showing the UI indicating you are off-origin counts as a "new top-level browsing context" (just not a normal browser window). On Chrome for Desktop, we plan to do (c).

I think the normative part of the spec (that applies to bounded scope; after #550 this will always apply) should be clear that (b) or (c) are both permitted.

Further problems. In the bounded case, the paragraph under §4 applies:

The user agent MUST navigate the application context as per [HTML]'s navigate algorithm with exceptions enabled. If the URL of the resource being loaded in the navigation is not within scope of the navigation scope of the application context's manifest, then the user agent MUST behave as if the application context is not allowed to navigate. This provides the ability for the user agent to perform the navigation in a different browsing context, or in a different user agent entirely. If during the handle redirects step of HTML's navigate algorithm the redirect URL is not within scope of the navigation scope of the application context's manifest, abort HTML's navigation algorithm with a SecurityError.

This has a few problems:

  1. As discussed on Properly define "navigation scope" and "within scope". #644, there is no "handle redirects" step of HTML's navigate algorithm. Not sure how to fix this.
  2. "with exceptions enabled". I'm not sure why this is here. Turning on exceptions makes navigation automatically fail with a SecurityError if it's not allowed to navigate (i.e., if it goes out of scope). That nullifies the next sentence: "This provides the ability for the user agent to perform the navigation in a different browsing context". No, it doesn't; if exceptions were disabled, then the navigation algorithm explicitly allows the UA to open in a new top-level browsing context. So I think "with exceptions enabled" should be removed, and similarly, the text for aborting with SecurityError should be reworked.

I think we just need some language that says the UA MUST create a new top-level browsing context when navigating off-scope, and somehow explain that this doesn't have to be a normal browser window; it can be within the application window but has to show the location bar (or does that language have to be non-normative?)

@kenchris
Copy link
Collaborator

I agree that this is a bit messy and inconsistent. I am also totally OK with banning a).

As Marcos wrote a lot of this original spec text, I wonder if he has comments.

Your last suggestion sounds really good and simple

@mgiuca
Copy link
Collaborator Author

mgiuca commented Jan 30, 2018

Yep happy to wait for @marcoscaceres 's input. Also note that I'm in no way an expert on the HTML navigate algorithm. I just skimmed it yesterday. So I'm not sure on the exact details of how this works, but the above two things seem broken to me.

@kenchris
Copy link
Collaborator

@marcoscaceres any comments? or should Matt just move ahead?

@marcoscaceres
Copy link
Member

I'm happy for the spec to follow Chrome's behavior, so long as the WebKit folks are also happy.

About the weird things that are currently specified (e.g., "with exceptions enabled"), they must have been in an older version of HTML.

Generally, if we can hook into HTML for navigation - or let HTML do the heavy lifting here, then we should work with HTML editors to achieve that.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Mar 9, 2018

OK I'll write up a draft to the Manifest that doesn't change HTML. If, during review, we think some of this belongs in HTML, we try moving it.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Mar 9, 2018

So "unbounded" scope is gone, which simplifies things a lot.

The main question I am trying to answer is: do we need a normative change to allow a navigation out-of-scope within the same browser context, as long as UI is shown to make it clear you are out of scope. TL;DR: No, we should not (at this time) make a normative change. Thus I will just reword the paragraph to make things clearer.

Long explanation

In my initial report, I detailed 3 cases marked (a), (b) and (c). I'm going to split (b) into two sub-cases because it's a bit more nuanced:

A navigation that would normally stay within the same context (e.g., a left-click on a non-targeted link) could, when in an app context:

  • a) Continue navigating within the same browser context, with no noticeable UI to indicate you're out of scope, or
  • b1) Continue navigating within the same browser context, but show some UI indicating that you're out of scope (e.g., a location bar or banner showing the out-of-scope origin), or
  • b2) Create a new browser context and navigate there, but have the new browser context exist within the app window, covering over the app's browser context, and show some UI indicating that you're out of scope (e.g., a location bar or banner showing the out-of-scope origin), or
  • c) Create a new browser context in a new window or browser tab, and navigate there.

Previously (before #647 removed unbounded scope), unbounded scope allowed the UA to perform any of the above, but RECOMMENDED (b1), (b2) or (c) (because (a) presents a security risk, as the user would not realise they are off-origin). The current text requires (b2) or (c), i.e., that a new browser context is created. You cannot navigate an application context out of scope, but you can create a new browsing context within the same application window (b2), or just bounce the user out to a separate browser tab (c), or even a different browser.

In Chrome for Desktop, we do (c). In Chrome for Android, we do (b2). A previous implementation on Chrome for Desktop was doing (b1) but we decided it was clearer to go with (c).

What I have been trying to think about is is there any material difference between (b1) and (b2), and if so, should we also allow (b1)? In other words, can the user or developer (of either the source or destination page) tell the difference between:

  • (b1), a simple navigation from in-app to out-of-app, which goes back to the in-app page when you press the back button, and
  • (b2), a new browsing context covering over the in-app page, which closes to reveal the in-app page when you press the back button.

The answer is "yes", for a couple of reasons:

  1. In (b2), the JavaScript code for the source page continues executing even though the page is hidden. (Even on Android where it is pushed into the background.) We tried using setInterval and the source page was able to console.log and even open new windows repeatedly while the page was hidden. In (b1), the source page does not exist while the destination page is open.
  2. In (b1), the destination page can see the source page in window.history, and can navigate back to it with window.history.back(). In (b2), the destination page has no history.
  3. In (b2), the destination page can see and communicate with the source page with window.opener (e.g., can postMessage to it, and learn lots of details if it is on the same origin). In (b1), there is no opener because the source page context no longer exists.
  4. In (b2), if Window.close is used, it closes the off-site browsing context, going back to the app. In (b1), Window.close closes the entire app browsing context. Discussed in crbug.com/771418.

So there are a few noticeable differences, and therefore, I think it's prudent to continue to disallow (b1). If a user agent wants to implement (b1) behaviour for out-of-scope navigations, we can discuss allowing this at a later time.

In the mean time, I will rewrite that paragraph to make it clearer what is allowed and what is not, but not change the semantics.

Thanks to @g-ortuno for helping with the above exploration.

Edited 2018-03-28, to add a fourth reason why b1 and b2 are different.

@kenchris
Copy link
Collaborator

kenchris commented Mar 9, 2018

This sounds pretty good and clear to me

@marcoscaceres
Copy link
Member

@mgiuca anything left to be done here?

@mgiuca
Copy link
Collaborator Author

mgiuca commented Apr 17, 2018

There is; updating the spec with non-normative changes mentioned above.

I've had this on my TODO list for some time. I'll get to it eventually (things have been very busy and will continue to be for the next 2 weeks). If you or someone else would like to take this, feel free.

@marcoscaceres
Copy link
Member

Oh, no problem. Sorry, I thought we had already added the above to the spec. Happy to wait, as I'm also traveling and busy with other things.

@lukekarrys lukekarrys mentioned this issue May 3, 2018
6 tasks
@mgiuca
Copy link
Collaborator Author

mgiuca commented May 22, 2018

Update: It seems that Chrome for Android recently landed changes [1] to effectively go from (b2) to (b1) in the above taxonomy. That is, out-of-scope navigations currently (in Chrome 66) open a new navigation context, but in a future version, they will navigate within the same navigation context.

This is in violation of the current spec text:

"The user agent MUST navigate the application context as per [HTML]'s navigate algorithm with exceptions enabled. If the URL of the resource being loaded in the navigation is not within scope of the navigation scope of the application context's manifest, then the user agent MUST behave as if the application context is not allowed to navigate. This provides the ability for the user agent to perform the navigation in a different browsing context, or in a different user agent entirely."

The reason for the change is that sites were broken when installed because they expect normal navigation behaviour off site (particularly for authentication flows where they go to a different origin for auth, and expect to redirect back). We've seen similar reports on Chrome OS and Safari [2] where the current spec text is followed.

Coming back to my last comment from March:

So there are a few noticeable differences, and therefore, I think it's prudent to continue to disallow (b1). If a user agent wants to implement (b1) behaviour for out-of-scope navigations, we can discuss allowing this at a later time.

I think maybe we want to change to (b1) behaviour. We should be opinionated about this, as opposed to allowing either. So the proposal would be to allow navigations outside of the scope, but require (or recommend, as strongly as possible from within spec-land) that UI is shown while out-of-scope to indicate that you are no longer within the app. Basically, what we had before for "unbounded scope", when you are off-origin.

[1] https://crrev.com/c/1050386 (Chrome 67) and https://crrev.com/c/1008735 (Chrome 68)
[2] #550 (comment)

mgiuca added a commit to mgiuca/manifest that referenced this issue Jul 5, 2018
Previously, user agents were required to prevent off-scope navigation (or open
it in a new top-level browsing context). Now, they are not allowed to do this,
as it broke existing sites not designed to be run under these conditions.

Replaced with a recommendation to show the URL when off-scope.

Closes w3c#646.
@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 5, 2018

OK, I've written the spec draft updates. This is complex so I'll re-summarize the above, and we realised there's also a few more cases (d) and (e).

The options that a User Agent might take when the site attempts to navigate to a URL out of scope are:

  • a) Continue navigating within the same browser context, with no noticeable UI to indicate you're out of scope
  • b) Navigate to the destination within the same app window, but show some UI indicating you're out of scope (e.g., a location bar or banner showing the out-of-scope origin)
    • b1) Navigate the same browser context to the new URL, as with normal HTML, but show UI whenever the page is out-of-scope.
    • b2) Create a new browser context and navigate there, but have the new browser context exist within the app window, covering over the app's browser context.
  • c) Create a new browser context in a new window or browser tab, and navigate there.
  • d) Open the URL in another browser (i.e., the operating system default browser).
  • e) Do nothing (behave as if "not allowed to navigate").

As discussed above, while (b1) and (b2) seem almost the same to the user, they have vastly different characteristics under the hood, and in fact (b2) breaks a lot of sites auth flows, as does (c), (d) and (e).

Current known implementations:

  • Chrome for Android: b1 (was b2 previously)
  • Firefox on Android: b2
  • Chrome OS: c
  • Safari on iOS: c

The current behaviour allows for (b2), (c), (d) or (e). The wording is a bit vague and broken; I've uploaded a pull request #700 to reword the existing functionality more precisely.

The new proposed behaviour in #701 switches to allow for (a) (strongly not recommended, though we can't mandate it), and (b1). This would require all existing implementations to update so we'll need a commitment to implement from at least a few vendors. However, I think it's a necessary change for this platform, to basically make navigation in an installed app work like navigation on the web. Otherwise, apps that navigate off-scope have subtly different behaviour when running in an installed app.

Note that this doesn't apply for target=_blank / window.open() navigations, only self-navigations. This allows user agents to implement either (c) or (d) for target=_blank navigations. (d) is a good option, because if we're pretending these installed apps are like native apps, when they want to open a new browser page, they should open it in the user's preferred browser, not whatever browser the app was installed from. The above proposal does not prevent (d), just (correctly) makes it only possible when the site requests to open the page externally.

@dominickng
Copy link
Collaborator

Note that Chrome for Android uses b1, not b2 as of version 67 or 68. No new browsing context is created for off-scope navigations; the UI is merely transitioned to show a URL bar.

mgiuca added a commit to mgiuca/manifest that referenced this issue Jul 5, 2018
Previously, user agents were required to prevent off-scope navigation (or open
it in a new top-level browsing context). Now, they are not allowed to do this,
as it broke existing sites not designed to be run under these conditions.

Replaced with a recommendation to show the URL when off-scope.

Closes w3c#646.
mgiuca added a commit to mgiuca/manifest that referenced this issue Jul 5, 2018
Previously, user agents were required to prevent off-scope navigation (or open
it in a new top-level browsing context). Now, they are not allowed to do this,
as it broke existing sites not designed to be run under these conditions.

Replaced with a recommendation to show the URL when off-scope.

Closes w3c#646.
@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 5, 2018

Thanks, updated. I'm also not sure what Edge uses, @boyofgreen @kirupac.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 6, 2018

I've updated Killer Marmot (our testing PWA) to prove why this needs to change. (Note: This change is not being made for compat with any particular implementation, though it happens to align to recent versions of Chrome on Android. It's because the currently specced behaviour breaks expectations of existing sites.)

Test site: https://killer-marmot.appspot.com/web/

Steps to test:

  1. Install this app and open it in a window.
  2. Click "Post!" under "POST-and-redirect".
  3. Should navigate to https://redirectonpost.appspot.com/, showing a white page with a countdown for 3 seconds.
  4. Should navigate back to Killer Marmot, which should read "I see you're back!"

Under the current spec text, step 3 must take place in a different browsing context (either in a separate window as we do on Chrome OS, or over the top of the app window, as Firefox for Android does and Chrome for Android used to do.) Step 4 must therefore take place in that secondary browsing context, or some complex non-standard machinery catches the redirect back and moves back into the original app context.

Under the proposed spec text, step 3 must take place within the original app context (but we highly recommend showing "redirectonpost.appspot.com" to highlight the off-scope origin). Step 4 should therefore naturally also take place within the original app context, with no origin display because you're back inside the app scope.

@kenchris
Copy link
Collaborator

kenchris commented Jul 6, 2018

Ah great, I was wondering about a test app for this.

On Chrome Canary on Android, it indeed shows the URL when navigating to the new page, but it also shows the URL when being back in the PWA.

@g-ortuno
Copy link

g-ortuno commented Jul 9, 2018

@kenchris I think that's a bug in Chrome. I've also been seen that behavior on other apps that navigate off scope.

@mgiuca
Copy link
Collaborator Author

mgiuca commented Jul 9, 2018

mgiuca added a commit to mgiuca/manifest that referenced this issue Jul 9, 2018
Previously, user agents were required to prevent off-scope navigation (or open
it in a new top-level browsing context). Now, they are not allowed to do this,
as it broke existing sites not designed to be run under these conditions.

Replaced with a recommendation to show the URL when off-scope.

Closes w3c#646.
mgiuca added a commit that referenced this issue Oct 9, 2018
Previously, user agents were required to prevent off-scope navigation (or open
it in a new top-level browsing context). Now, they are not allowed to do this,
as it broke existing sites not designed to be run under these conditions.

Replaced with a recommendation to show the URL when off-scope.

Closes #646.
christianliebel pushed a commit to christianliebel/manifest that referenced this issue May 27, 2020
Previously, user agents were required to prevent off-scope navigation (or open
it in a new top-level browsing context). Now, they are not allowed to do this,
as it broke existing sites not designed to be run under these conditions.

Replaced with a recommendation to show the URL when off-scope.

Closes w3c#646.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants