Skip to content

Commit

Permalink
Strictly enforce exclusive semantics for <details name>.
Browse files Browse the repository at this point in the history
This change thoroughly enforces that at most one <details> element in a
group established by the name attribute is open at a given time.   This
requires that we remove the open attribute during insertion into the DOM
in some cases, and also that we remove the open attribute for some
mutations of the name attribute.

This suppresses DOM mutation events (but not toggle events, which are
asynchronous) for the removals of the open attribute that result from
insertion.

This is based on discussion in (among other places):
openui/open-ui#786
openui/open-ui#812

Bug: 1444057
Change-Id: I91313662f6fb005b461717445a69294bcb4d1a59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4829108
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1195601}
  • Loading branch information
dbaron authored and Lightning00Blade committed Dec 11, 2023
1 parent b27fc1a commit 314d118
Showing 1 changed file with 43 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@
assert_array_equals([first.open, second.open, third.open], [expected_first, expected_second, expected_third], description);
}

assert_states(true, false, true, "initial states from open attribute");
assert_states(true, false, false, "initial states from open attribute");
first.open = true;
assert_states(true, false, true, "non-mutation doesn't change state");
assert_states(true, false, false, "non-mutation doesn't change state");
second.open = true;
assert_states(false, true, false, "mutation closes multiple open elements");
third.setAttribute("open", "");
Expand Down Expand Up @@ -148,19 +148,19 @@
}));
}
assert_array_equals(mutation_event_received_ids, []);
assert_element_states(elements, [1, 0, 1, 1], "states before mutation");
assert_element_states(elements, [1, 0, 0, 0], "states before mutation");
elements[1].open = true;
if (mutation_event_received_ids.length == 0) {
// ok if mutation events are not supported
} else {
assert_array_equals(mutation_event_received_ids, ["e0", "e2", "e3", "e1"],
"removal events received in tree order, followed by addition event");
assert_array_equals(mutation_event_received_ids, ["e0", "e1"],
"removal event followed by addition event");
}
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, ["e1", "e0", "e2", "e3"], "toggle events received after awaiting promises");
}, "mutation event and toggle event order matches tree order");
assert_array_equals(toggle_event_received_ids, ["e3", "e2", "e1", "e0"], "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
// relevant only because of Mutation Events. If mutation events (for
Expand Down Expand Up @@ -203,9 +203,9 @@
element.addEventListener("DOMSubtreeModified", listener);
}
assert_array_equals(received_ids, []);
assert_element_states(elements, [1, 0, 1], "states before mutation");
assert_element_states(elements, [1, 0, 0], "states before mutation");
elements[1].open = true;
assert_array_equals(received_ids, ["e0", "e2", "e1"],
assert_array_equals(received_ids, ["e0", "e1"],
"removal events received in tree order, followed by addition event, despite changes to name during mutation event");
assert_element_states(elements, [0, 1, 0], "states after mutation");
}, "interaction of open attribute changes with mutation events");
Expand Down Expand Up @@ -326,4 +326,38 @@
}, `exclusivity enforcement with attachment scenario ${scenario}`);
}

promise_test(async t => {
container.innerHTML = `
<details name="a" id="e0" open></details>
<details name="a" id="e1"></details>
<details name="b" id="e2" open></details>
`;
let elements = [ document.getElementById("e0"),
document.getElementById("e1"),
document.getElementById("e2") ];

assert_element_states(elements, [1, 0, 1], "states before first mutation");
elements[2].name = "a";
assert_element_states(elements, [1, 0, 0], "states after first mutation");
elements[0].name = "c";
elements[2].open = true;
assert_element_states(elements, [1, 0, 1], "states before second mutation");
elements[0].name = "a";
assert_element_states(elements, [0, 0, 1], "states after second mutation");
}, "handling of name attribute changes");

promise_test(async t => {
container.innerHTML = `
<details name="a" id="e0" open></details>
<details name="a" id="e1" open></details>
<details open name="a" id="e2"></details>
`;
let elements = [ document.getElementById("e0"),
document.getElementById("e1"),
document.getElementById("e2") ];

assert_element_states(elements, [1, 0, 0], "states after insertion by parser");
}, "closing as a result of parsing doesn't depend on attribute order");


</script>

0 comments on commit 314d118

Please sign in to comment.