search

whatwg / dom

DOM Standard
https://dom.spec.whatwg.org/
Other
1.58k stars 294 forks source link

Strange behavior with `insertBefore` vs `appendChild` and transitions #880

Open dead-claudia opened 4 years ago

dead-claudia commented 4 years ago

When using insertBefore with transitions and the FLIP technique, I get very strange results across browsers.

The only difference is the way the DOM nodes are rearranged, which is what confuses me about this.

Code for each `insertBefore`: ```html
  • 0
  • 1
  • 2
  • 3
``` `appendChild`: ```html
  • 0
  • 1
  • 2
  • 3
```

I've reproduced this behavior explained above on each of the following platforms:

Relevant framework bugs I've filed, before I narrowed it down further to this:

I suspect it's a bug in all three of those listed browsers, but I'm filing an issue here in case it's actually a spec issue or if a spec note is necessary for this.

annevk commented 4 years ago

Maybe @emilio or @tabatkins knows why this happens. This should probably be filed over at https://github.com/w3c/csswg-drafts/issues/new though.

dead-claudia commented 4 years ago

@annevk Filed: https://github.com/w3c/csswg-drafts/issues/5334

annevk commented 4 years ago

Thanks to the explanation there I recalled https://github.com/whatwg/html/issues/5484. @josepharhar here's a concrete example as to what a move primitive might improve.

dead-claudia commented 4 years ago

If it helps, this is in the context of FLIP animations for list move transitions.

dead-claudia commented 4 years ago

Question: would it be web-compatible to do this, or would this be too breaking?

Add the following operation to trees:

To move a child within a tree to a given index is to move the child within the tree's children from its current position to before the item in the tree's children with the given index prior to moving, without adding or removing. If the index is the size of the tree's children, this moves it to the end of the tree instead.

To move a child within a tree to the end is to move the child within the tree's children from its current position to after its last child, without adding or removing.

Note: moves can only be performed whenever the child is a child of that tree, and by extension, moves cannot be performed on trees with empty children.

This is just spec jargon for "don't do things that happen when you add or remove elements". In implementations, this would still necessitate removing and then re-adding, but only as an implementation detail.

Amend the adopt algorithm to this:

To adopt a node into a document with a given parent, run these steps:

  1. Let oldDocument be node's node document.
  2. If node's parent is not parent, then remove node.
  3. If document is not oldDocument, then: [...]

Amend step 4 of document.adoptNode(node) likewise to pass a parent of null.

Replace steps 7.2-7.3 of insert with the following:

  1. If node's parent is parent, then:
    1. If child is non-null, then move node within parent to child's index.
    2. Otherwise, move node within parent to the end.
  2. Otherwise, if child is null, then append node to parent's children.
  3. Otherwise, insert node into parent's children before child's index.

Amend step 11 of replace to be this:

  1. If child's parent is not parent, then: [...]

Amend replace all to move children rather than removing them if their parents are parent (as defined in the algorithm), and update its callees like .replaceChildren(...nodes) and .textContent appropriately.

The precise update needed is omitted as it would be rather involved and would span multiple otherwise unrelated algorithms, but that's because the removal would need moved from .replaceChildren(...) to the core algorithm, and in order to do that, the algorithm itself would need to change from operating on a single node at a time to operating on a list of nodes (what it should've been using the whole time IMHO).

To assess risk, you would want to check for elements that 1. would be moved to any position other than the end according to this change, 2. have active transitions, animations, or mutation observers, and 3. for transitions, didn't have their styles synchronously calculated (for .getBoundingClientRect() mainly) within the current rendering frame. AFAICT, there's no other way to observe this change from JS, and custom element callbacks aren't fired on children changes (only mutation observers are).

josepharhar commented 4 years ago

Thanks for writing this up! It sounds like moving elements around within the same parent without clobbering state could be a huge win! I'm interested to hear what the react and preact maintainers say about it in the whatwg/html thread.

yosunkayag commented 4 years ago

Npc list?

Get Outlook for Androidhttps://aka.ms/ghei36

From: Joey Arhar notifications@github.com Sent: Tuesday, August 11, 2020 6:58:25 AM To: whatwg/dom dom@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [whatwg/dom] Strange behavior with insertBefore vs appendChild and transitions (#880)

Thanks for writing this up! It sounds like moving elements around within the same parent without clobbering state could be a huge win! I'm interested to hear what the react and preact maintainers say about it in the whatwg/html thread.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/whatwg/dom/issues/880#issuecomment-671710025, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AO6Z7OQYPWPZCNCNLC3YKQLSAC6WDANCNFSM4PB7QJXQ.

dead-claudia commented 4 years ago

@josepharhar Thanks! Of course, I just maintain Mithril - React and Preact of course have different concerns as they're implemented slightly differently, but I suspect it should work for them if I were to take an educated guess. (Preact is a lot like us but with fewer internal optimizations based on a cursory look, and React uses linked lists for everything IIUC.)

josepharhar commented 4 years ago

@sebmarkbage @marvinhagemeister @developit I sort of already asked in this comment, but would @isiahmeadows's suggestion here of not clobbering state when moving around nodes within one parent solve react's and preact's issues with state being lost when moving nodes?

marvinhagemeister commented 4 years ago

@josepharhar Sorry for being late to answer! The improvements outlined here would help us tremendously for Preact 👍 Same goes for the other issue.

annevk commented 4 years ago

@isiahmeadows I don't think that would be web-compatible as the side effects of insertion and removal would not be triggered. We'd also want different mutation records for moving vs insertion/removal.