Fire toggle events using a microtask

Instead of firing the async popover toggle events using a regular task, they should be fired at microtask timing to be faster. As suggested in the issue, this patch also uses microtasks for the dialog element's close event and the details element's toggle event: whatwg/html#9046 Bug: 1485980 Change-Id: I70569a36e6447283a5101117693d235020ebb4a6
web-platform-tests · Sep 28, 2023 · 7fd6bd7 · 7fd6bd7
1 parent a230a40
commit 7fd6bd7
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 26 deletions.
diff --git a/html/semantics/interactive-elements/the-details-element/name-attribute.tentative.html b/html/semantics/interactive-elements/the-details-element/name-attribute.tentative.html
@@ -159,7 +159,7 @@
   assert_element_states(elements, [0, 1, 0, 0], "states after mutation");
   assert_array_equals(toggle_event_received_ids, [], "toggle events received before awaiting promises");
   await Promise.all(toggle_event_promises);
-  assert_array_equals(toggle_event_received_ids, ["e3", "e2", "e1", "e0"], "toggle events received after awaiting promises, including toggle events from parser insertion");
+  assert_array_equals(toggle_event_received_ids, ["e0", "e3", "e2", "e1"], "toggle events received after awaiting promises, including toggle events from parser insertion");
 }, "mutation event and toggle event order");
 
 // This function is used to guard tests that test behavior that is

diff --git a/.../semantics/interactive-elements/the-details-element/toggle-event-microtask.tentative.html b/.../semantics/interactive-elements/the-details-element/toggle-event-microtask.tentative.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:[email protected]">
+<link rel=help href="https://github.com/whatwg/html/issues/9046">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<details>hello</details>
+
+<script>
+promise_test(async () => {
+  // Toggling details elements during parsing may have special behavior.
+  await new Promise(resolve => window.onload = resolve);
+
+  const details = document.querySelector('details');
+  let gotToggleEvent = false;
+  details.ontoggle = () => gotToggleEvent = true;
+
+  // Asserting inside the microtask callback causes a timeout, so pull the
+  // value out instead.
+  const gotToggleEventInTime = await new Promise(resolve => {
+    details.open = true;
+    queueMicrotask(() => {
+      resolve(gotToggleEvent);
+    });
+  });
+
+  assert_true(gotToggleEventInTime);
+}, '<details> toggle events should be fired at microtask timing.');
+</script>
diff --git a/html/semantics/interactive-elements/the-details-element/toggleEvent.html b/html/semantics/interactive-elements/the-details-element/toggleEvent.html
@@ -10,10 +10,6 @@
   <summary>Lorem ipsum</summary>
   <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>
 </details>
-<details id=details2 open>
-  <summary>Lorem ipsum</summary>
-  <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>
-</details>
 <details id=details3 style="display:none;">
   <summary>Lorem ipsum</summary>
   <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>
@@ -28,7 +24,7 @@
   <summary>Lorem ipsum</summary>
   <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>
 </details>
-<details id=details8 open>
+<details id=details8>
   <summary>Lorem ipsum</summary>
   <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>
 </details>
@@ -47,7 +43,6 @@
 </details>
 <script>
   var t1 = async_test("Adding open to 'details' should fire a toggle event at the 'details' element, with 'oldState: closed' and 'newState: open'"),
-  t2 = async_test("Adding open to 'details' and then removing open from that 'details' should fire only one toggle event at the 'details' element, with 'oldState: closed' and 'newState: closed'"),
   t3 = async_test("Adding open to 'details' (display:none) should fire a toggle event at the 'details' element, with 'oldState: closed' and 'newState: open'"),
   t4 = async_test("Adding open to 'details' (no children) should fire a toggle event at the 'details' element, with 'oldState: closed' and 'newState: open'"),
   t6 = async_test("Adding open to 'details' and then removing open from that 'details' and then again adding open to that 'details' should fire only one toggle event at the 'details' element, with 'oldState: closed' and 'newState: closed'"),
@@ -57,7 +52,6 @@
   t10 = async_test("Setting open=false on a closed 'details' element should not fire a toggle event at the 'details' element"),
 
   details1 = document.getElementById('details1'),
-  details2 = document.getElementById('details2'),
   details3 = document.getElementById('details3'),
   details4 = document.getElementById('details4'),
   details6 = document.getElementById('details6'),
@@ -82,14 +76,6 @@
   });
   details1.open = true; // opens details1
 
-  details2.ontoggle = t2.step_func_done(function(evt) {
-    assert_equals(evt.oldState, "closed");
-    assert_equals(evt.newState, "closed");
-    assert_false(details2.open);
-    testEvent(evt);
-  });
-  details2.open = false; // closes details2
-
   details3.ontoggle = t3.step_func_done(function(evt) {
     assert_equals(evt.oldState, "closed");
     assert_equals(evt.newState, "open");
@@ -147,6 +133,7 @@
     assert_false(details8.open);
     testEvent(evt)
   });
+  details8.open = true;
   details8.removeAttribute('open'); // closes details8
 
   window.details9TogglePromise.then(t9.step_func(() => {

diff --git a/html/semantics/interactive-elements/the-dialog-element/close-event-microtask.tentative.html b/html/semantics/interactive-elements/the-dialog-element/close-event-microtask.tentative.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:[email protected]">
+<link rel=help href="https://github.com/whatwg/html/issues/9046">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<dialog>dialog</dialog>
+
+<script>
+promise_test(async () => {
+  const dialog = document.querySelector('dialog');
+  dialog.showModal();
+
+  let gotClose = false;
+  dialog.addEventListener('close', () => gotClose = true);
+
+  // Asserting inside the microtask callback causes a timeout, so pull the
+  // value out instead.
+  const gotCloseInTime = await new Promise(resolve => {
+    dialog.close();
+    queueMicrotask(() => {
+      resolve(gotClose);
+    });
+  });
+  assert_true(gotCloseInTime);
+}, '<dialog> close events should be fired at microtask timing.');
+</script>
diff --git a/html/semantics/interactive-elements/the-summary-element/activation-behavior.html b/html/semantics/interactive-elements/the-summary-element/activation-behavior.html
@@ -117,7 +117,6 @@
   summary.click();
   summary.click();
   summary.click();
-  Promise.resolve().then(() => summary.click());
 
 }, "toggle events should be coalesced even when using the activation behavior of a summary");
 

diff --git a/html/semantics/popovers/toggle-event-microtask.tentative.html b/html/semantics/popovers/toggle-event-microtask.tentative.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:[email protected]">
+<link rel=help href="https://github.com/whatwg/html/issues/9046">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div popover=auto>popover</div>
+
+<script>
+promise_test(async () => {
+  const popover = document.querySelector('div');
+
+  let gotToggle = false;
+  popover.addEventListener('toggle', () => gotToggle = true);
+
+  // Asserting inside the microtask callback causes a timeout, so pull the
+  // value out instead.
+  let gotToggleInTime = await new Promise(resolve => {
+    popover.showPopover();
+    queueMicrotask(() => {
+      resolve(gotToggle);
+    });
+  });
+  assert_true(gotToggleInTime, 'opening toggle');
+
+  gotToggle = false;
+  gotToggleInTime = false;
+
+  gotToggleInTime = await new Promise(resolve => {
+    popover.hidePopover();
+    queueMicrotask(() => {
+      resolve(gotToggle);
+    });
+  });
+  assert_true(gotToggleInTime, 'closing toggle');
+}, 'popover toggle events should be fired at microtask timing.');
+</script>
diff --git a/...ing/processing-model-2/unhandled-promise-rejections/promise-rejection-events-onerror.html b/...ing/processing-model-2/unhandled-promise-rejections/promise-rejection-events-onerror.html
@@ -4,6 +4,7 @@
 <script src="/resources/testharnessreport.js"></script>
 <link rel="help" href="https://html.spec.whatwg.org/#runtime-script-errors">
 <link rel="help" href="https://html.spec.whatwg.org/#unhandled-promise-rejections">
+<body>
 <script>
 'use strict';
 setup({
@@ -37,11 +38,12 @@
 
 // This function queues a task in "DOM manipulation task source"
 function queueTask(f) {
-  var d = document.createElement("details");
-  d.ontoggle = function() {
+  const script = document.createElement('script');
+  script.setAttribute('src', ' ');
+  script.addEventListener('error', () => {
+    script.remove();
     f();
-  };
-
-  d.setAttribute("open", "");
+  });
+  document.body.appendChild(script);
 }
 </script>
diff --git a/...s/scripting/processing-model-2/unhandled-promise-rejections/promise-rejection-events.html b/...s/scripting/processing-model-2/unhandled-promise-rejections/promise-rejection-events.html
@@ -5,4 +5,5 @@
 <script src="/resources/testharnessreport.js"></script>
 <link rel="help" href="https://html.spec.whatwg.org/#unhandled-promise-rejections">
 
+<body>
 <script src="support/promise-rejection-events.js"></script>
diff --git a/...pting/processing-model-2/unhandled-promise-rejections/support/promise-rejection-events.js b/...pting/processing-model-2/unhandled-promise-rejections/support/promise-rejection-events.js
@@ -884,11 +884,13 @@ if ('document' in self) {
 // context, but not in workers.
 function queueTask(f) {
   if ('document' in self) {
-    var d = document.createElement("details");
-    d.ontoggle = function() {
+    const script = document.createElement('script');
+    script.setAttribute('src', ' ');
+    script.addEventListener('error', () => {
+      script.remove();
       f();
-    };
-    d.setAttribute("open", "");
+    });
+    document.body.appendChild(script);
   } else {
     // We need to fix this to use something that can queue tasks in
     // "DOM manipulation task source" to ensure the order is correct