Rewrite node insertion algorithm to match the spec #35999
Conversation
xiaochengh
force-pushed
the
fix-node-insert
branch
from
5d34884 to
950e695
Compare
xiaochengh
force-pushed
the
fix-node-insert
branch
from
950e695 to
840db26
Compare
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#51993) with upstreamable changes. |
xiaochengh
force-pushed
the
fix-node-insert
branch
from
840db26 to
4cfb53f
Compare
|
🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#51993). |
xiaochengh
force-pushed
the
fix-node-insert
branch
from
4cfb53f to
cd8d1f0
Compare
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#51997) with upstreamable changes. |
xiaochengh
force-pushed
the
fix-node-insert
branch
from
cd8d1f0 to
fd482c7
Compare
|
🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#51997). |
xiaochengh
force-pushed
the
fix-node-insert
branch
3 times, most recently
from
7f9848f to
0ac6dc7
Compare
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#52040) with upstreamable changes. |
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52040). |
|
@xiaochengh Is this one ready to review? |
|
@mrobinson yes |
components/script/dom/node.rs
Outdated
| if let Some(child) = child { | ||
| if !parent.ranges_is_empty() { | ||
| let index = child.index(); | ||
| // Steps 2.1-2. |
There was a problem hiding this comment.
I think this step number is inaccurate now?
There was a problem hiding this comment.
Right. Let me remove this step number since the two steps are merged into the same utility function.
components/script/dom/node.rs
Outdated
|
|
||
| // Step 8. If suppress observers flag is unset, then queue a tree mutation record for parent | ||
| // with nodes, « », previousSibling, and child. |
There was a problem hiding this comment.
Hrm. Is this actually happening? It might be worth a comment explaining why if not or a TODO if we are missing it.
There was a problem hiding this comment.
Yes, it's done after the children_changed call. I moved it to right next to the lines.
What confused me is that children_changed is called only when observers are unsuppressed, which is different from the spec.
Signed-off-by: Xiaocheng Hu <[email protected]>
xiaochengh
force-pushed
the
fix-node-insert
branch
from
5001338 to
3139446
Compare
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52040). |
Signed-off-by: Xiaocheng Hu <[email protected]>
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52040). |
Per spec, adoption of new node should be done while inserting the node. This patch moves the call site of
adoptto insideinsertto match it.It also rewrites some existing code to better match the spec without any behavioral changes.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors