Make "is unloading" more explicit in navigation

[domenic]

This replaces vague text about "if the prompt to unload algorithm is
being run" and the like for "unload" with an explicit check of the
"unload counter". The "unload counter" is a rename of the
"ignore-opens-during-unload counter", which was already used for similar
explicit checks in document.open().

This results in the same check being done everywhere, which appears to
match at least some implementations better. Previously, for example,
"traverse the history by a delta" did not check for beforeunload, only
unload.

See also #2666 for potential followup.

• At least two implementers are interested (and none opposed):
  • Chrome implements this
  • Safari implements this
• Tests are written and can be reviewed and commented upon at:
  • Test that history traversal is disallowed during unload + beforeunload web-platform-tests/wpt#28909 (Firefox fails these)
• Implementation bugs are filed:
  • Chrome: N/A
  • Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1710530
  • Safari: N/A

(See WHATWG Working Mode: Changes for more details.)

---

/browsing-the-web.html ( diff )
/dynamic-markup-insertion.html ( diff )
/history.html ( diff )

[annevk]

My knowledge of this is a bit limited and I've always found it hard to test as well. One worry here is that "prompt to unload" and "unload a document" are separate algorithms and this seems to unify them from the point of view of the navigate algorithm. Perhaps an implementer can review this?

[domenic]

Well, they're already unified in navigate, but I guess in "traverse the history" we currently only check if "unload a document" is running, and not whether "prompt to unload" is running. This PR checks both in both cases.

@lucasgadani would you able to review this and compare with Chrome's implementation? Since our PR preview isn't working, I'll note that this modifies https://html.spec.whatwg.org/#traverse-the-history-by-a-delta step 5 (by also making it bail out if "prompt to unload" is running) and https://html.spec.whatwg.org/#navigate step 4 and 5 (but the modification there should be only editorial).

[domenic]

Another location that we could change to use this, with slight normative impacts, would be https://html.spec.whatwg.org/#stop-document-loading

[domenic]

I've checked the Chromium code and almost all locations use the same check:

• navigate algorithm
• stop document loading
• traverse history by a delta
• document open steps: uses the same check as elsewhere sometimes but earlier it can use an explicit ignore opens during unload counter.

Furthermore, referring to #2666, all of these checks except the last one do not seem to use a counter. It's possible that reentrancy is only possible with the document.open() case?

I think this should suffice for consolidating everything. However I'd like to keep things as a counter for now until we've looked into things in detail.

I don't think tests are going to be very possible for the consolidation change I propose here since the distinction between unload and prompt to unload depends heavily on user prompting, which often is not even shown since browsers sometimes exercise their freedom to ignore prompting for iframes and popups.

[annevk]

1. If browsers are consistent about not prompting in certain scenarios we could enshrine that.
2. It seems that "prompt to unload" and "unload" are called after each other (sometimes with many steps in between) and fire different events. Even if no prompting happens other bits might still run?

[domenic]

If browsers are consistent about not prompting in certain scenarios we could enshrine that.

They are not :(. Generally Firefox seems to prompt less for iframes, and Chrome less for (unfocused) popups.

It seems that "prompt to unload" and "unload" are called after each other (sometimes with many steps in between) and fire different events. Even if no prompting happens other bits might still run?

I'll check on that; my impression was that the intermediate steps cannot run author code, but I might be wrong.

[domenic]

The case where prompt to unload and unload are separated is navigation; there prompt to unload happens early, but unload happens late (after the response is received). However, I can't figure out how to use this in a way that might distinguish the change being proposed here from the previous one.

[domenic]

What should we do with this? I think it's a nice improvement, but I can understand if it's too much of a normative change to make without somehow getting better test coverage. If we require that for moving it forward, I'll close that and maybe one day we can come back to it.

[annevk]

@lucasgadani @mystor @mattwoodrow thoughts on this?

[lucasgadani]

I think this is an improvement over the existing text.

As far as prompting to unload, Chrome will only prompt to unload documents that have received user gestures (main frame or iframes). So the user has to interact with the document in order to receive the prompt to unload.

[domenic]

@annevk I'd like to resurrect this. (App history is trying to figure out what guards its navigations and history traversals should have, and having a solid foundation for the existing spec seems like the right first step.)

I did find one testable difference between the existing spec and the proposal here: the existing spec disallows history traversal during unload, but not during beforeunload. Chrome, and this proposal, disallows it during both events. Firefox appears to allow history traversal during both events. Do you think Firefox would be willing to follow?

(We'll see what Safari does when wpt.fyi finishes running...)

[domenic]

I tested manually on Safari using my partner's computer and it appears Safari matches Chrome and this proposal, yay.

[annevk]

I like this!

I've asked @smaug---- for a second opinion.

[smaug----]

@domenic does Chrome allow history navigations if one posts a task in beforeunload event listener to do history.back()?
Or if the task to do a history navigation is pending when beforeunload is called?

[domenic]

It appears to depend on the task. setTimeout() tasks can cause the back to go through, while postMessage() tasks do not. I assume this is due to different prioritization of the history traversal task source vs. the timer task source vs. the posted message task source.

[domenic]

I found the code that matches that. Indeed, we do an additional "IsNavigationAllowed" (i.e. is beforeunload/unload running) check in the posted task, for two total on history delta traversal. So I guess we should probably add that to the spec here? I'm unsure about tests though given that it's task-source dependent...

[domenic]

Let's merge this as a clear step in the right direction. I'd be interested in followups if we can nail them down.

[smaug----]

Check in posted tasks? Do you mean in postMessage case but not in setTimeout? So is the check done when message is being posted?

[annevk]

I filed #6672 as follow-up.