whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.06k stars 2.65k forks source link

Navigables reloading does not work #9869

Open kalenikaliaksandr opened 11 months ago

kalenikaliaksandr commented 11 months ago

What is the issue with the HTML Standard?

There is a bug in the navigable's reload procedure.

  1. reload calls apply the history step.
  2. apply the history step attempt to populate the history entry's document on step 12.6.7
  3. 12.6.7 says that completionSteps need to do following: set to queue a global task on the navigation and traversal task source given navigable's active window to run afterDocumentPopulated.
  4. queued task to run afterDocumentPopulated is never executed because it is queued on navigable's active window which is corresponding to document that is no longer active. (the document is no longer active because active session history's document got replaced during population of session history entry caused by pending reloading).
Kedarnath-Rothe commented 11 months ago

Please assign this issue to me

domenic commented 11 months ago

I guess this depends on what time you believe "navigable's active window" is accessed, right? Is it accessed during step 12.6.7, when we're declaring the steps? (In which case we run into the problem you describe.) Or is it accessed at the time the steps are run? (In which case we get the new post-reload window.)

I would say that we generally treat these sorts of steps as "closures", so it closes over navigable, but the actual access of "navigable's active window" happens inside the steps and thus only at the time the steps are run.

Still, I guess we can make this a bit clearer.

kalenikaliaksandr commented 11 months ago

I would say that we generally treat these sorts of steps as "closures", so it closes over navigable, but the actual access of "navigable's active window" happens inside the steps and thus only at the time the steps are run.

I might be missing something, but in this specific case, you always need to get an active window before the callback is queued on the event loop because it will define to which document the queued task belongs (global parameter in https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-global-task)

kalenikaliaksandr commented 5 months ago

Responding to https://github.com/whatwg/html/pull/9901#issuecomment-1790112168

In Ladybird's implementation I ended up solving that by passing cloned active SHE into "populate session history entry" algorithm and then patching active SHE in unloading callback with all fields that could have been modified in "populate session history entry" (See https://github.com/SerenityOS/serenity/commit/91377f3ab98ba4e225295330220fcf5aad8f2f1b).