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

navigate events on traversal #114

Closed
domenic opened this issue May 14, 2021 · 3 comments · Fixed by #129
Closed

navigate events on traversal #114

domenic opened this issue May 14, 2021 · 3 comments · Fixed by #129

Comments

@domenic
Copy link
Collaborator

domenic commented May 14, 2021

In trying to spec appHistory.back(), I realized that there's a whole world of complexity around traversals that we seem to have forgotten, which is that history traversals can navigate multiple windows. Should all of them get navigate events? Which of them should be cancelable? Which should have canRespond true?

See also #78 and #32 which touch on some related questions, but no issue so far has tackled the multi-window issue I believe. There is currently a spec ("Modify the traverse the history by a delta algorithm" in https://wicg.github.io/app-history/#navigate-algorithm-patches) but it doesn't appear to acknowledge the possibility of multiple windows being traversed, so I need to rewrite it.

Recapping some guiding principles from previous discussions:

  • If you get traversed and can either cancel it or respondWith() to replace it with a same-document nav, then you should get a navigate event. Conversely, if you can do neither of those things, then don't fire the navigate event, since it's not actionable.

  • Same-document traversals always have canRespond true, and thus always fire navigate events. A SPA needs to be able to react to back-navigations just as much as it does to link clicks or other such things.

  • Cross-document traversals must have canRespond false because converting those into same-document ones is just confusing.

  • Developers are interested in canceling user-initiated traversals (Cancelling UI initiated navigations (back/forward) #32) although currently we've specced this as disallowed.

I'll add my own additional principle, which hasn't been previously discussed: you shouldn't be able to cancel a traversal (using event.preventDefault()) unless "you" initiated it. I.e. if you're getting navigated because some other frame called history.back(), then the event should be uncancelable.

I think this leads to a model like the following:

  1. Let currentWindow be the Window of the History or AppHistory object, or of the top-level browsing context being displayed while the user is pressing the back button.

  2. Go to the browser process to figure out what windows a given traversal will affect, and whether the traversal will be same- or cross-document.

  3. For each such Window w:

    1. Let canRespond be true if it's same-document, false otherwise.

    2. Let cancelable be true if this navigation is programatically-initiated and w = currentWindow; false otherwise. (We may also set it to true in some user-initiated cases per Cancelling UI initiated navigations (back/forward) #32 in the future.)

    3. If canRespond is true or cancelable is true:

      1. Queue a task on w to fire the navigate event with cancelable and canRespond set appropriately.

      2. If the event is canceled, then bail out of the whole traversal (like when the user allows a beforeunload prompt to cancel the traversal), and reject the return value of appHistory.back() or whatever with an "AbortError" DOMException.

      3. If they called respondWith(), then...

        1. Track the promise given to respondWith. Maybe all the promises should be Promise.all()ed together to feed back into the return value of appHistory.back()? Or maybe only the one where w = currentWindow? I lean toward the latter.

        2. I don't think we need to do anything else? The history traversal was already underway, and it was already going to be same-document. So we just let it go through?

Ideally we'd have this be done in a single centralized algorithm, probably as patches to Jake's "apply the history step". In this case we'd probably do these... after the beforeunload prompt portion of "apply the history step"?? (Step 13 currently.)

Then history.back() and appHistory.back() and user-initiated back button pressing all go through this centralized algorithm.

How does this sound, @jakearchibald @natechapin? I know in the short term in Chromium our implementation is firing the navigate event entirely within the renderer process, only for appHistory.back(), but as the vision for the ultimate architecture, does this make sense?

@domenic
Copy link
Collaborator Author

domenic commented May 19, 2021

@jakearchibald @natechapin ping for thoughts on this plan

@jakearchibald
Copy link

Sorry for the slow reply!

I'll add my own additional principle, which hasn't been previously discussed: you shouldn't be able to cancel a traversal (using event.preventDefault()) unless "you" initiated it. I.e. if you're getting navigated because some other frame called history.back(), then the event should be uncancelable.

Do you feel the same about a link-click which targets some other frame? I guess it would be inconsistent otherwise.

Ideally we'd have this be done in a single centralized algorithm, probably as patches to Jake's "apply the history step". In this case we'd probably do these... after the beforeunload prompt portion of "apply the history step"?? (Step 13 currently.)

Agreed, although I think it comes before the beforeunload stuff, since we only want to do beforeunload if we're changing document.

Unfortunately the beforeunload thing happens in multiple places, because:

Link click:

  1. Can/should I perform this navigation? If no, abandon.
  2. Create new history entry, and traverse to it.

History traversal:

  1. Can/should all of the navigations be performed? If no, abandon.
  2. Start the navigations.

I couldn't figure out a single place for beforeunload, and I expect it'll be the same for dispatching navigate. Although, it'll probably be in the same places, so it could be abstracted a bit.

@domenic
Copy link
Collaborator Author

domenic commented May 21, 2021

Do you feel the same about a link-click which targets some other frame? I guess it would be inconsistent otherwise.

Hmm. Currently the readme says that those are only censored if the navigation is cross-document and initiated from a cross origin-domain window. I remember thinking through that now in some detail in #75. Especially the same-document case is important to fire navigate on. So probably we should align on that for this case too.

Agreed, although I think it comes before the beforeunload stuff, since we only want to do beforeunload if we're changing document.

Good point.

I couldn't figure out a single place for beforeunload, and I expect it'll be the same for dispatching navigate. Although, it'll probably be in the same places, so it could be abstracted a bit.

Yep, in general we often have this issue with shared infrastructure betweeen traversal and navigation. I'm happy to duplicate for now and abstract later.

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

Successfully merging a pull request may close this issue.

2 participants