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

Meta: a list of side effects that would be impacted by state-preserving move #1270

Open
noamr opened this issue Apr 3, 2024 · 7 comments
Open

Comments

@noamr
Copy link
Collaborator

noamr commented Apr 3, 2024

What is the issue with the DOM Standard?

In support of #1255, this issue tries to capture the exhaustive list of behaviors that would be impacted by an state-preserving atomic move operation (e.g. element.moveBefore(). It is based on references to insertion steps and removing steps.

Potentially skipped side effects

Potentially modified side effects

Reflection

  • Mutation observer: potentially reflect that there was a move
  • Custom elements: see reactions. Perhaps add a movedCallback, that has a default to call disconnectedCallback and connectedCallback?

Noops

@WebReflection
Copy link

Mutation observer: potentially reflect that there was a move

It's already easy to fail behind MutationRecords order (added VS removed, which to check first? I check removed first and update the logic synchronously for libraries which goal is to reflect connected and disconnected behavior) so this would be awesome to tackle and I see only two ways to do so:

  • both removed and added nodes lists refer to the same node with an extra moved boolean value (safe as it's undefined and considered always false to date, if ever even checked)
  • movedNodes list enters the game with optionally a from and to references for the nodes

To be honest, this logic could be easily polyfilled too by checking the parentNode and isConnected properties of any removed node but if handled natively people would be surely happier ... the move operation is key to avoid tons of clean-up and events dispatching for something that maybe needs again, once reconnected, to bootstrap tons of stuff so it's not only the browser insertions/removals steps we all win, if less, it's a huge ecosystem of libraries that do a lot of work on disconnected and connected so I hope this MO related story will be as successful as the idea behind moving.

Custom elements ... Perhaps add a movedCallback, that has a default to call disconnectedCallback and connectedCallback?

That would be the least surprise and fully backward compatible so I am overly 👍 with the movedCallback idea and its defaulting to current behavior: disconnected + connected

This helps Web Components libraries authors to migrate while browsers adoption increase so it feels like a no-brainer to me and I hope it will be welcomed by others too.

@Kaiido
Copy link
Member

Kaiido commented Apr 4, 2024

I believe this would also affect being-rendered, whose exhaustive implications I am not sure.

  • At least <object> and <embed>1 would be affected.
  • It might have effect on <img> element that allow auto-size, though I'm really not sure here.
  • That might also affect <select> and <input> elements on which showPicker() has been called, keeping some pickers alive.

Footnotes

  1. Though the specs for <embed> are a bit weird and ask the checks are made against the state at the first step of the event loop. But browsers do agree that moving it around in the DOM does trigger a reload, even if the move has been completed in the same EL iteration.

@noamr
Copy link
Collaborator Author

noamr commented Apr 4, 2024

I believe this would also affect being-rendered, whose exhaustive implications I am not sure.

  • At least <object> and <embed>1 would be affected.
  • It might have effect on <img> element that allow auto-size, though I'm really not sure here.
  • That might also affect <select> and <input> elements on which showPicker() has been called, keeping some pickers alive.

Footnotes

  1. Though the specs for <embed> are a bit weird and ask the checks are made against the state at the first step of the event loop. But browsers do agree that moving it around in the DOM does trigger a reload, even if the move has been completed in the same EL iteration.

Thanls, added these!

@noamr
Copy link
Collaborator Author

noamr commented Apr 9, 2024

I created a demo that tries to capture all these scenarios: https://state-preserving-atomic-move.glitch.me

@Kaiido
Copy link
Member

Kaiido commented Aug 10, 2024

Another one that came to my mind from whatwg/html#10521, which is more a direct consequence of not reloading the iframes anymore: in update the rendering the list of documents that share the same container document are supposed to be in tree order. Currently that order can't change during this task, with atomic moves, it could be changed anywhere during the task.

It seems that the current implementation in Chrome (weirdly) ignores the move even outside of the task: https://jsfiddle.net/vr7f958d/

@domfarolino
Copy link
Member

in update the rendering the list of documents that share the same container document are supposed to be in tree order. Currently that order can't change during this task, with atomic moves, it could be changed anywhere during the task.

First, I'm not really sure that the ordering can change anywhere during the task in a way that it couldn't before our API. moveBefore() doesn't really introduce that kind of ability, if I understand your point correctly.

Second:

It seems that the current implementation in Chrome (weirdly) ignores the move even outside of the task: https://jsfiddle.net/vr7f958d/

Yes it seems that Chromium is wrong here. Basically, it seems like the order in which rAF callbacks are dispatched on same-event-loop same-parent-document-rooted iframes is based on frame insertion order (something closer to this) than actual DOM node order. But that doesn't seem to be unique to moveBefore(). I think the same inconsistency is observable with insertBefore() or any other classic method of insertion. Consider https://jsfiddle.net/6xsdc3qr/2/, which shows the mismatch in ordering between Chrome and the spec, for iframes whose DOM node order does not match their insertion order. WebKit agrees with Chromium in this example, by the way, so I think this is a legacy problem, and something generally unrelated to moveBefore().

@Kaiido
Copy link
Member

Kaiido commented Aug 14, 2024

moveBefore() doesn't really introduce that kind of ability, if I understand your point correctly.

The thing that moveBefore introduces and that could matter here, is that before, moving an <iframe> or an <object> or any other element embedding another accessible document would unload said document and clear its list of animation frame callbacks. So before, if you moved such an element during an animation frame, I suppose that its document would get removed from the list and the new one might not be part of it yet (though the specs don't seem to really handle this case, so I'm not sure what's really supposed to happen). With moveBefore the documents aren't unloaded and so it's important whether the list docs is a static list or dynamic, and that could now be observed.

Yes it seems that Chromium is wrong here. [...]

Interesting that this "bug" can be observed even today, still I think that moveBefore through its ability to keep alive the previous docs could introduce new observable discrepancies like this. E.g. I don't think it's possible today to test that the list of docs keeps its order during each steps of the task, nor is it clear if it should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants