back-forward cache: specify interaction with unload & beforeunload

[rakina]

A fork from #5744.

One of the biggest differences between Firefox & Safari (and soon, Chrome) seems to be on how it handles unload & beforeunload:

• Firefox does not cache pages with unload/beforeunload handlers
• Safari caches pages with unload/beforeunload handlers, won't fire unload handlers on navigations where the old page gets bfcached, and will only fire beforeunload handlers on cross-origin navigations (CMIIW)
• Chrome caches pages with unload/beforeunload handlers, won't fire unload handlers on navigations where the old page gets bfcached, and will fire beforeunload on all navigations.

I believe Safari & Chrome decides to cache these pages because there are already bfcache-friendly replacements for unload (pagehide & visibilitychange event), with some mitigation steps on Chrome. Firefox is not caching when these event handlers are present because of possible compatibility issues for sites that depend on unload (and the beforeunload name implies it will be called only when unload is fired next? hmm..).

I wonder if we can maybe converge on what to do here (probably in the future, after explicit opt-out is widely used?) or at least specify that unload handlers might not be fired, and beforeunload might be fired even if a page won't get unloaded in the end.

For other bfcache-related issues, see #5880

cc @annevk @smaug---- @mystor @cdumez @beidson @hober @altimin @xharaken @fergald

[cdumez]

Safari caches pages with unload/beforeunload handlers, won't fire unload handlers on navigations where the old page gets bfcached, and will only fire beforeunload handlers on cross-origin navigations (CMIIW)

That seems very surprising. WebKit should be firing beforeunload for all navigations as far as I know (and we have tests that seem to confirm that).

[altimin]

Safari caches pages with unload/beforeunload handlers, won't fire unload handlers on navigations where the old page gets bfcached, and will only fire beforeunload handlers on cross-origin navigations (CMIIW)

That seems very surprising. WebKit should be firing beforeunload for all navigations as far as I know (and we have tests that seem to confirm that).

Hm, I was able to repro this pretty consistently, however I haven't had a chance to test Safari 14 yet.
Could you try this: https://back-forward-cache-tester.glitch.me/?beforeunload=1&persistent_logs=1 and let me know if it works for you or not?

For Safari 12 on my Mac is saw beforeunload() firing for same-origin navigations, but not for cross-origin ones.
For Safari 13 on my iPad I didn't saw beforeunload() firing at all.

Please let me know if I'm missing something.

[cdumez]

I just tried that site on Safari 14 on macOS. I do not see beforeunload getting fired on link navigations either same-site or cross-site. I do see beforeunload getting fired for URL bar navigations, either same-site or cross-site. URL bar navigations are pretty-different in Safari, thus the difference in behavior. beforeunload events firing for one and not the other is likely an oversight on our part though but I would need to dig into the code to figure out why. I don't think we meant to fire the beforeunload event at all.

[rakina]

Just to update this thread: Here's the doc detailing web-exposed behavior of Chrome's bfcache (+ comparing it w/ Firefox & Safari in most of them, though some parts are still missing - would be great to get comments from relevant parties here :) ).

[domenic]

Currently the spec uses salvagable to specify the Gecko behavior. (Or at least, something close to it.)

I think we should remove this from the spec, since it is being actively and intentionally violated by 2/3 browsers. And even if we do remove it, then Gecko is still compliant (i.e. we have 3/3 browsers compliant); in that world, Gecko just fails to bfcache in some cases even when the spec allows them to do so.

Then we should specify behavior for beforeunload/unload. It sounds like from the above that the intention of Safari is to match Chrome's behavior, so we have at least some tentative agreement? So we could probably proceed to specify this, or at least create a pull request to start a more concrete discussion.

[rakina]

Oh thanks, did not know that bit exists!
So both running the beforeunload handler and running the unload handler will set salvageable to false and eventually it will trigger "discard a Document".

I think we should:

• Remove the part where dispatching beforeunload will set "salvageable" to false.
• Make running the unload handler optional in "unload a Document". Something like "if the user agent does not intend to keep the Document alive, it should run the unload event and set salvageable to false"? If we decide to spec bfcache-ineligibility we can throw a check for that in here too.
• Maybe rename "unload a Document" and also "prompt to unload" to not actually include "unload" in its name? Something like "navigate away from a Document" and "prompt to navigate"? That does not include cases like closing a tab though. Maybe "leave"?

If that sounds OK, I can send a PR :)

[annevk]

If you don't have salvageable, how do you determine PageTransitionEvent's persisted?

The changes @rakina suggests seem reasonable at a glance.

[domenic]

+1 to Rakina's suggested changes. I suggest when saying "keep the Document alive" you mention "in a [session history entry]" specifically.

I think "unload" is still a reasonable name, even if the events aren't always fired? I might be missing something though.

[rakina]

Cool! I made #5889, PTAL :)

On renaming the steps that refers to the "unload" name, hmm, I guess in my mind if a document is still alive, that means it's not really "unloaded" as we won't "load" it again after reusing it (so calling "unload a Document" might not actually result in the document getting "unloaded"). But maybe that's just my preconception. Anyways, we can probably leave that problem for another day.

[domenic]

On renaming the steps that refers to the "unload" name, hmm, I guess in my mind if a document is still alive, that means it's not really "unloaded" as we won't "load" it again after reusing it (so calling "unload a Document" might not actually result in the document getting "unloaded")

Oh, yes, I see what you mean. I tend to agree now. But yeah, we could tackle that separately.