State-preserving atomic move integration #10657

domfarolino · 2024-09-29T19:28:45Z

This PR is a supplementary PR to whatwg/dom#1307. It provides the HTML-overrides for the DOM Standard's "moving steps" extension, to allow pieces of the HTML Standard to react to atomic DOM moves without the use of the "insertion steps" or "removing steps", which by default, intentionally fail to preserve most state. The changes in this PR are inspired by the audit carried out in https://docs.google.com/document/d/1qfYyvdK4zhzloABKeh0K1lHPm-SpnEcsWEE9UdDuoMk/edit.

At least two implementers are interested (and none opposed):
Tests are written and can be reviewed and commented upon at:
- https://github.com/web-platform-tests/wpt/tree/master/dom/nodes/moveBefore/tentative (need to mark as non-tentative)
Implementation bugs are filed:
- Chromium: https://issues.chromium.org/issues/40150299
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1923880
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=281223
MDN issue is filed: …
The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

💥 Error: Wattsi server error 💥

PR Preview failed to build. (Last tried on Nov 4, 2024, 1:08 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.

🔗 Related URL

      <!DOCTYPE html>
      <html>
      <head>
          <meta name="viewport" content="width=device-width, initial-scale=1">
          <meta name="robots" content="noindex">
          <style>body,html{height:100%;margin:0}body{display:flex;align-items:center;justify-content:center;flex-direction:column;-webkit-font-smoothing:antialiased;text-rendering:optimizeLegibility}p{text-align:center;font-family:-apple-system,BlinkMacSystemFont,Segoe UI,Roboto,Oxygen,Ubuntu,Cantarell,Fira Sans,Droid Sans,Helvetica Neue,sans-serif;color:#000;font-size:14px;margin-top:-50px}p.code{font-size:24px;font-weight:500;border-bottom:1px solid #e0e1e2;padding:0 20px 15px}p.text{margin:0}a,a:visited{color:#aaa}</style>
      </head>
      <body>
      <p class="code">
        Error
      </p>
      <p class="text">
        We encountered an error when trying to load your application and your page could not be served. Check the logs for your application in the App Platform dashboard.
      </p>
        <div style="display:none;">
          <h1>
    upstream_reset_before_response_started{connection_termination} (503 UC)      </h1>
          <p data-translate="connection_timed_out">App Platform failed to forward this request to the application.</p>
      </div>
      </body>
      </html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

… flag

This reverts commit f6e9fbe.

smaug---- · 2024-11-08T01:19:44Z

source

+
+    <ol>
+     <li><p>If the <span>form-associated element</span>'s <span>parser inserted flag</span> is set,
+     then return.</p></li>


I assume this special case is here because something similar happens currently when removing and adding such element back?

See the first row in https://docs.google.com/document/d/1qfYyvdK4zhzloABKeh0K1lHPm-SpnEcsWEE9UdDuoMk/edit?tab=t.0 for this in general.

However regarding this condition specifically, if I remember correctly it was just copied from the insertion steps to preserve the behavior of remove+insert during a move. Looking at it again, I'm unsure how this is supposed to work though.

Imagine a form-associated element with a form owner being removed from the DOM. There are two cases:

During removal, that element gets its form owner reset because it hits the conditions in the "HTML element removing steps". This unsets the parser inserted bit. Next time that element gets inserted into the DOM, we'll reset the form owner again, always to the nearest form element.

During removal, that element does not get its form owner reset. The parser-inserted bit remains set even though the element is disconnected. When it's inserted next, we trip the insertion condition "If the parser inserted flag is set", and we do not reset the form owner, ever.

It isn't clear to me when (2) above can ever happen. That is, when during removal will we not "reset the form owner" of a form-associated element that has a non-null form owner? The condition reads:

If the form-associated element has a form owner and the form-associated element and its form owner are no longer in the same tree, then reset the form owner of the form-associated element.

So it seems that (during removal) we'd avoid resetting the owner if the element is still in the same tree as its form owner. But removing makes an element disconnected, so I don't think it could ever be in the same tree as anything. I'm left to think that:

There's a bug in the current HTML element removing steps

Removed form-associated elements with non-null form owners are always reset (during removal)

We don't need the parser inserted condition in the HTML element moving steps

If I am wrong about (1)/(2) in the immediately-above list, then we may still need the parser inserted condition in the moving steps.

I don't know much about forms and their associated mechanics but I feel like @zcorpan does and would maybe be willing to take a look here.

But removing makes an element disconnected, so I don't think it could ever be in the same tree as anything.

Why not? If both are a child of some element that gets removed, they're all in the same tree still. Just a tree that's disconnected. (We should probably stop using "same tree" though and instead always do root checks. That's clearer.)

I don't think we should have this parser-inserted check here though. That's a legacy thing and the parser won't call move. So we can just always reset for moves.

smaug---- · 2024-11-08T01:23:19Z

source

@@ -72810,7 +72874,10 @@ customElements.define("x-foo", class extends HTMLElement {
   <li><p>When it <span>becomes disconnected</span>, its <code data-x="">disconnectedCallback</code>
   is called, with no arguments.</p></li>

-   <li><p>When it is <span data-x="concept-node-adopt">adopted</span> into a new document, its <code
+   <li><p>When it is <span data-x="concept-node-move-ext">moved</span>, its <code
+   data-x="">connectedMoveCallback</code> is called, with no arguments.</p></li>


Shouldn't callback get the old subtree owner as an argument, so that one can know easily where one was.

This would require holding a reference to the old tree which might be gone by the time the CE callbacks are fired. Perhaps we can consider this as a future enhancement? I think we should be able to do that in a compatible way.

annevk

I don't think you mentioned connectedMoveCallback everywhere connectedCallback is mentioned (and it's relevant).

https://html.spec.whatwg.org/#custom-element-reactions
https://html.spec.whatwg.org/#element-definition (this one seems especially important)
It could do with some examples as well.

annevk · 2024-11-26T12:41:45Z

source

+
+    <ol>
+     <li><p>If the <span>form-associated element</span>'s <span>parser inserted flag</span> is set,
+     then return.</p></li>


But removing makes an element disconnected, so I don't think it could ever be in the same tree as anything.

Why not? If both are a child of some element that gets removed, they're all in the same tree still. Just a tree that's disconnected. (We should probably stop using "same tree" though and instead always do root checks. That's clearer.)

annevk · 2024-11-26T12:42:47Z

source

+
+    <ol>
+     <li><p>If the <span>form-associated element</span>'s <span>parser inserted flag</span> is set,
+     then return.</p></li>


I don't think we should have this parser-inserted check here though. That's a legacy thing and the parser won't call move. So we can just always reset for moves.

annevk · 2024-11-26T12:44:13Z

source

+   <span data-x="relevant mutations">relevant mutation</span> for <var>movedNode</var>.</p></li>
+  </ol>
+
+  <p>The <code>img</code> <span>HTML element removing steps</span>, given <var>movedNode</var> and


removedNode

annevk · 2024-11-26T12:47:03Z

source

+   <li><p>When it is <span data-x="concept-node-move-ext">moved</span>, its <code
+   data-x="">connectedMoveCallback</code> is called, with no arguments.</p></li>
+
+    <li><p>When it is <span data-x="concept-node-adopt">adopted</span> into a new document, its <code


Indentation.

annevk · 2024-11-26T12:47:52Z

source

@@ -72952,6 +73019,35 @@ customElements.define("x-foo", class extends HTMLElement {
   data-x="concept-custom-element-definition-lifecycle-callbacks">lifecycle callbacks</span> with
   key <var>callbackName</var>.</p></li>

+   <li>
+    <p>If <var>callbackName</var> is "<code data-x="">connectedMoveCallback</code>" and
+    <var>callback</var> is null, then:


Modern style:

Suggested change

<var>callback</var> is null, then:

<var>callback</var> is null:

Did we agree that this fallback behavior needs to be there? I'm still a bit skeptical I notice.

Yes, we did discuss this. Some existing custom elements might have existing connectedCallback logic that relies on checking hierarchy.

domfarolino and others added 13 commits September 29, 2024 15:19

State-preserving atomic move integration

28cc642

Details + autofocus + frame

8d41fc5

Add HTML element moving steps override

7107603

media + option + optgroup

ea1dfd4

Revert most or all usages of state-preserving atomic move in progress…

1339991

… flag

Spacing

6f77c67

Remove extra newlines

5bbf7c6

Use move steps instead of the flag

5fe636f

Add <img> and <source> relevant mutations back

8f95485

Move enqueing to DOM spec, but include fallback

1f1b3ff

Listed form element owner reset

412d6a3

Form owner reset note

bdf58da

Remove Document flag

b746a3d

domfarolino mentioned this pull request Oct 10, 2024

Introduce moveBefore() state-preserving atomic move API whatwg/dom#1307

Open

19 tasks

noamr added 2 commits October 31, 2024 16:57

Reset focus when refocusing document with a moved element

f6e9fbe

Revert "Reset focus when refocusing document with a moved element"

b12ef4f

This reverts commit f6e9fbe.

smaug---- reviewed Nov 8, 2024

View reviewed changes

domfarolino requested a review from annevk November 20, 2024 21:52

domfarolino marked this pull request as ready for review November 20, 2024 21:52

domfarolino added impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation addition/proposal New features or enhancements labels Nov 20, 2024

annevk reviewed Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

State-preserving atomic move integration #10657

State-preserving atomic move integration #10657

domfarolino commented Sep 29, 2024 •

edited by pr-preview bot

Loading

smaug---- Nov 8, 2024

domfarolino Nov 20, 2024 •

edited

Loading

annevk Nov 26, 2024

annevk Nov 26, 2024

smaug---- Nov 8, 2024

noamr Nov 20, 2024

annevk left a comment

annevk Nov 26, 2024

annevk Nov 26, 2024

annevk Nov 26, 2024

annevk Nov 26, 2024

annevk Nov 26, 2024

annevk Nov 26, 2024

noamr Nov 26, 2024

	<var>callback</var> is null, then:
	<var>callback</var> is null:

State-preserving atomic move integration #10657

Are you sure you want to change the base?

State-preserving atomic move integration #10657

Conversation

domfarolino commented Sep 29, 2024 • edited by pr-preview bot Loading

💥 Error: Wattsi server error 💥

Choose a reason for hiding this comment

domfarolino Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domfarolino commented Sep 29, 2024 •

edited by pr-preview bot

Loading

domfarolino Nov 20, 2024 •

edited

Loading