Remove quadratic behavior from clone a node #1334
Conversation
Corresponding DOM PR: whatwg/dom#1334.
dom.bs
Outdated
| <li><p>For each <var>child</var> of <var>node</var>'s <a for=Element>shadow root</a>'s | ||
| <a for=tree>children</a>, in <a>tree order</a>: <a>clone a node and append</a> given | ||
| <var>child</var>, <var>copy</var>'s <a for=Element>shadow root</a>, <var>document</var>, and | ||
| <var>subtree</var>. |
There was a problem hiding this comment.
In Gecko shadow root attaching happens after going through children.
And the same seems to happen in Chrome too https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=689;drc=fe99dee25a6b65dbe6c9e97099334e9c027a24a1;bpv=0;bpt=1
https://mozilla.pettay.fi/moztests/shadowclone.html
The order of slotchange events is the same in Firefox and Chrome (h6, div, span), Safari has - I was told - div, span, h6.
There was a problem hiding this comment.
Yeah it is, but is that the result of cloning order or something else? As per the discussion in #695 there's already a difference between engines.
I can restore the original order here though. That seems better for now either way.
Strictly speaking this should be an editorial change, but as it's fairly significant not marking it as such. This improves integration with HTML as well. As part of this change we move the shadow tree behavior under creating the element clone as that is a more logical fit. Unfortunately subtree has to be passed everywhere as HTML needs it for its template element logic. That's also why clone a node's children is exported. Corresponding HTML PR: SOON. Fixes #1219. Closes #1220.
annevk
force-pushed
the
annevk/clone-a-node
branch
from
c88a25d to
da4e899
Compare
|
I've cleaned it up a bit more. Now only "clone a single node" is a separate algorithm. "Clone a single node" is not exported so other specifications cannot make use of it. @smaug---- I'd appreciate another review. I hope it's all good now. I'll align the HTML PR with this as well. |
|
|
||
| <li><p>Run any <a>cloning steps</a> defined for <var>node</var> in | ||
| <a>other applicable specifications</a> and pass <var>node</var>, <var>copy</var>, and | ||
| <var>subtree</var> as parameters. |
There was a problem hiding this comment.
Gecko runs cloning steps as part of cloning a single node, and I think webkit does that too https://searchfox.org/wubkat/rev/72adbb9f99534551a728acb270d4f111da5e9515/Source/WebCore/dom/Element.cpp#648,655,663
and chrome too https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=755;drc=f5e280b6c11f41545154266cd2317d3c730e9b30;bpv=0;bpt=1
Corresponding DOM PR: whatwg/dom#1334.
Main difference is that a chunk of the "clone a node" steps are pulled out into a "clone a single node" algorithm. Reflects these spec PRs: whatwg/dom#1332 whatwg/dom#1334 Though this code is quite old so there may also be older spec changes included here.
Main difference is that a chunk of the "clone a node" steps are pulled out into a "clone a single node" algorithm. Reflects these spec PRs: whatwg/dom#1332 whatwg/dom#1334 Though this code is quite old so there may also be older spec changes included here.
Strictly speaking this should be an editorial change, but as it's fairly significant not marking it as such. This improves integration with HTML as well.
All the arguments are exported as HTML needs them as well. And they are named parameters now for clarity.
Corresponding HTML PR: whatwg/html#10859.
Fixes #1219. Closes #1220.
Preview | Diff