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

Should appHistory.navigate()'s returned promise settle for cross-document navigations? #95

Closed
domenic opened this issue Apr 14, 2021 · 13 comments
Labels
change A proposed change to the API, not foundational, but deeper than the surface

Comments

@domenic
Copy link
Collaborator

domenic commented Apr 14, 2021

Consider a case where you do

appHistory.navigate("other-url").then(onFulfilled, onRejected);

and you have no navigate event handler to convert this into a same-document navigation. Here onFulfilled and onRejected will never be called, because by the time the navigate goes through, your document has been unloaded, so there's no way for onFulfilled and onRejected to run. Makes sense.

Of course, if you do that but then add a navigate handler which converts the navigation into a same-document navigation using event.respondWith(), then one of your handlers will get called, after the promise passed to respondWith() settles. That's the main use case for the returned promise.

Now consider the following case, again with no navigate event handlers:

frames[0].appHistory.navigate("other-url").then(onFulfilled, onRejected);

Here, because we're navigating a subframe, it's very conceivable that onFulfilled or onRejected could be called, at some point after the cross-document navigation of the subframe finishes. (When, exactly? The load event? DOMContentLoaded? pageshow? readystatechange? Similar issues to #31.)

Should this work?

Our tentative answer is "no": cross-document navigations should never settle the promise. It seems like a good deal of work to implement and it's outside the usual use cases of app history which are really supposed to be about your own frame.

On the other hand, I've written a lot of test code which does the equivalent of

iframe.src = "foo";
await new Promise(r => iframe.onload = r);

which could be nice to replace (for same-origin iframes) with

await iframe.contentWindow.appHistory.navigate("foo");

See also whatwg/html#5725; /cc @jakearchibald @annevk. So hmm. Maybe it would be nice.

@annevk
Copy link

annevk commented Apr 15, 2021

Unfortunately I don't have a "do this" answer, but I do have some thoughts. 😊

What happens with bfcache? Does the promise settle when you go back to the page that created it?

I'm not sure how serious @jkarlin was in shivanigithub/http-cache-partitioning#2 (comment) but that was a proposal to not have load events for cross-origin navigations. (It's not clear if that's actually feasible though as you can still tell when frame.contentDocument becomes inaccessible.)

In any event, we shouldn't introduce a signal that's currently not observable, such as DOMContentLoaded, pageshow, or readystatechange.

@jakearchibald
Copy link

jakearchibald commented Apr 15, 2021

This is an issue for portals too, but I think @mikewest is unhappy exposing the contentDocument-becomes-null timing more generally WICG/portals#191 (comment).

@domenic
Copy link
Collaborator Author

domenic commented May 12, 2021

@jakearchibald, @annevk, @smaug---- and I discussed this today. There are basically two problem cases:

  • A top-level page uses appHistory.navigate() and then goes into bfcache. Then the user goes back. When the page comes back to life, what happens to the promise?
  • A page manipulates another same-origin page, e.g. frames[0].appHistory.navigate(). What happens to the promise?

We discussed a few options:

  • The promise remains forever-pending (in both cases). This is the spec's current behavior.
  • The promise fulfills after navigation "finishes" in some sense, e.g. the new page's load event or its document updating (which, @annevk was surprised to learn, are different).
    • This is not great for the bfcache case because the promise handler won't actually run until the page is navigated back to it. So if you do appHistory.navigate(url).then(onFulfilled), at the time onFulfilled actually runs, the current URL is no longer url.
  • The promise fulfills at pagehide timing. That is, we consider the navigation "finished" at the point where we're committed to unloading the document and moving on.

We all ended up liking this last option a good bit.

@jakearchibald
Copy link

If the navigation results in a 204/5 or content-disposition, there's no pagehide, so I guess we'd fulfill as soon as we have the headers.

@annevk
Copy link

annevk commented May 17, 2021

Interesting, that provides a new source of timing. You'd also get that with downloads. Unclear if harmful. What do we do for non-HTTP(S) schemes?

@jakearchibald
Copy link

Interesting, that provides a new source of timing.

Right now, if you window.open(url) and it turns out to be a download, the window is closed (in Chrome and Firefox at least), so that timing is measurable.

@annevk
Copy link

annevk commented May 17, 2021

That only gives information if you know it's a download because url's response having COOP would yield the same result. (And that's also what I'd like us to start doing for custom schemes, act as if they had COOP.)

@domenic
Copy link
Collaborator Author

domenic commented May 20, 2021

I'd like a 204/205/download to result in a rejected promise, since the navigation never actually completes. But I guess that still reveals new information, hmm...

@annevk
Copy link

annevk commented May 21, 2021

Note that you can observe it through setInterval() as well so I'm not sure it's highly problematic in this case.

But we also need to solve custom schemes. And there are various cases there:

  • Maps to HTTPS
  • Maps to OS

And then browsers currently behave differently depending on what context is navigated. My point of view is that regardless of what context is navigated, a custom scheme always (acts as if it) opens a new top-level COOP context to do the navigation. In case it maps to HTTPS the navigation would happen there. In case it maps to OS you wouldn't really have a new context (unless window.open() was used I suppose). (There's an HTML issue on this.)

I think that navigate() should probably be able to navigate to custom schemes as otherwise it's incomplete, though we could also split that out into its own function (given the different behavior and given that certain options wouldn't apply to them) or add it later and restrict navigate() for now.

@domenic
Copy link
Collaborator Author

domenic commented Aug 19, 2021

A very similar issue came up while implementing/speccing dispose events.

Briefly, if you navigate #1 -> #2 -> back -> #3, all within the same document, you will get a dispose event on the entry for #2.

But if you navigate ?1 -> ?2 -> back -> ?3, you will not get a dispose event on the ?1 document's AppHistoryEntry for ?2, because the ?1 document isn't informed about app history changes when you leave it.

We might similarly want to fire dispose on forward-pruned entries around pagehide timing?

@domenic
Copy link
Collaborator Author

domenic commented Aug 19, 2021

We might similarly want to fire dispose on forward-pruned entries around pagehide timing?

Although, that's confusing because normally dispose fires after appHistory.entries()/appHistory.current has been updated. And we're not going to update appHistory.entries()/appHistory.current until the navigation actually settles, by which time the document is completely gone.

(Modulo bfcache... that's a separate issue which I won't derail the thread by getting into.)

@domenic
Copy link
Collaborator Author

domenic commented Aug 19, 2021

Tentative conclusion: rename dispose to beforedispose and fire it before the entries update in same-document cases, plus at pagehide time in cross-document cases.

@domenic
Copy link
Collaborator Author

domenic commented Nov 3, 2021

With regard to the last few comments, we went back to dispose after deciding that adding another pagehide-timing event is probably not a good idea, especially given that nobody had concrete use cases for doing additional cleanup when the page was being unloaded entirely.

Overall at this point I am relatively happy with a never-settled promise for cross-document navigations. Even in the bfcache case, where in theory we could resolve it after you arrive back at the document, it seems to confusing to do so. And having it resolve at pagehide time could be confusing since location.href / appHistory.current is not updated at that point, like it would be in same-document cases.

So I'll close this. We can discuss the downloads and 204 case in #135.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change A proposed change to the API, not foundational, but deeper than the surface
Projects
None yet
Development

No branches or pull requests

3 participants