Make <selectmenu>'s parts search always a flat tree traversal that sk…

…ips nested <selectmenu>s. Per an OpenUI resolution [1], a <selectmenu>'s search for parts should be a flat tree traversal that doesn't descend into nested <selectmenu> or <select> elements. This change updates <selectmenu> to use the resolved-upon behavior. As a consequence, the parts chosen depend only on tree structure, not on the order of insertion for dynamically inserted parts. [1] See https://www.w3.org/2021/06/17-openui-irc Bug: 1209219,1191121 Change-Id: Ia68c782b7972317b8e6abdcfff5e52ece34a3ed0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3053078 Reviewed-by: Mason Freed <[email protected]> Reviewed-by: Ionel Popescu <[email protected]> Commit-Queue: Dan Clark <[email protected]> Cr-Commit-Position: refs/heads/master@{#908088}
valerioadm · Aug 3, 2021 · b2622d1 · b2622d1
1 parent 05ab32f
commit b2622d1
Show file tree

Hide file tree

Showing 3 changed files with 180 additions and 0 deletions.
diff --git a/html/semantics/forms/the-selectmenu-element/selectmenu-nested.tentative.html b/html/semantics/forms/the-selectmenu-element/selectmenu-nested.tentative.html
@@ -0,0 +1,94 @@
+<!DOCTYPE html>
+<html lang="en">
+<title>HTMLSelectMenuElement Test: nested selects</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+
+<selectmenu id="selectMenu0">
+  <popup slot="listbox" part="listbox">
+    <selectmenu id="nested0">
+      <option id="child1">one</option>
+      <option id="child2">two</option>
+    </selectmenu>
+    <option id="child3">three</option>
+  </popup>
+</selectmenu>
+
+<selectmenu id="selectMenu1">
+  <popup slot="listbox" part="listbox">
+    <select>
+      <option>one</option>
+      <option>two</option>
+    </select>
+    <option>three</option>
+  </popup>
+</selectmenu>
+
+<selectmenu id="selectMenu2">
+  <div slot="button">
+    <selectmenu id="nested2">
+      <div slot="button" part="button" id="selectMenu2-button0">button0</div>
+      <option id="nested2-option1">one</option>
+    </selectmenu>
+    <div part="button" id="selectMenu2-button1">button1</div>
+  </div>
+  <option>two</option>
+</selectmenu>
+
+<script>
+  function clickOn(element) {
+    const actions = new test_driver.Actions();
+    return actions.pointerMove(0, 0, {origin: element})
+      .pointerDown({button: actions.ButtonType.LEFT})
+      .pointerUp({button: actions.ButtonType.LEFT})
+      .send();
+  }
+
+  promise_test(async () => {
+    const selectMenu0 = document.getElementById("selectMenu0");
+    const nested0 = document.getElementById("nested0");
+    const child2 = document.getElementById("child2");
+    assert_equals(selectMenu0.value, "three", "Options nested in another <selectmenu> should not get controller code from outer <selectmenu>");
+    await clickOn(selectMenu0);
+    assert_true(selectMenu0.open);
+    assert_false(nested0.open);
+
+    await clickOn(nested0);
+    assert_true(nested0.open);
+
+    await clickOn(child2);
+    assert_false(nested0.open);
+    assert_equals(nested0.value, "two");
+    assert_true(selectMenu0.open, "click on option in inner <selectmenu> should not close outer <selectmenu>");
+    assert_equals(selectMenu0.value, "three", "click on option in inner <selectmenu> should not change value of outer <selectmenu>");
+  }, "A <selectmenu> shouldn't apply controller code to parts nested in a <selectmenu> child");
+
+  promise_test(async () => {
+    const selectMenu1 = document.getElementById("selectMenu1");
+    assert_equals(selectMenu0.value, "three");
+  }, "A <selectmenu> shouldn't apply controller code to parts nested in a <select> child");
+
+  promise_test(async () => {
+    const selectMenu2 = document.getElementById("selectMenu2");
+    const nested2 = document.getElementById("nested2");
+    const button0 = document.getElementById("selectMenu2-button0");
+    const button1 = document.getElementById("selectMenu2-button1");
+    const nested2Option1 = document.getElementById("nested2-option1");
+    assert_false(selectMenu2.open);
+    assert_false(nested2.open);
+
+    await clickOn(button0);
+    assert_false(selectMenu2.open, "Clicking the button of a nested <selectmenu> should not open the outer <selectmenu>");
+    assert_true(nested2.open, "Clicking the button of a nested <selectmenu> should open the outer <selectmenu>");
+
+    await clickOn(nested2Option1);
+    assert_false(nested2.open);
+
+    await clickOn(button1);
+    assert_true(selectMenu2.open);
+    assert_false(nested2.open);
+  }, "A nested button part in a nested <selectmenu> shouldn't get controller code even if it comes first in document order");
+</script>
diff --git a/html/semantics/forms/the-selectmenu-element/selectmenu-parts-structure.tentative.html b/html/semantics/forms/the-selectmenu-element/selectmenu-parts-structure.tentative.html
@@ -44,6 +44,23 @@
   </div>
 </selectmenu>
 
+<selectmenu id="selectMenu3">
+  <div slot="button" id="selectMenu3-button-slot">
+    <div part="button" id="selectMenu3-button0">button0</div>
+  </div>
+  <option>one</option>
+</selectmenu>
+
+<selectmenu id="selectMenu4">
+  <div slot="button" part="button" id="selectMenu4-button0">button0</div>
+  <div slot="listbox" id="selectMenu4-listbox-slot">
+    <popup part="listbox" id="selectMenu4-listbox0">
+      <option>one</option>
+      <option id="selectMenu4-option2">two</option>
+    </popup>
+  </div>
+</selectmenu>
+
 <script>
   function clickOn(element) {
     const actions = new test_driver.Actions();
@@ -109,4 +126,53 @@
     await clickOn(selectMenu2Child4);
     assert_equals(selectMenu2.value, "four", "Clicking an <option> that is a descendant of a valid listbox part should update the value");
   }, "To receive listbox part controller code, an element labeled as a listbox must not be a descendant of the button part in a flat tree traversal");
+
+  promise_test(async () => {
+    const selectMenu3 = document.getElementById("selectMenu3");
+    const selectMenu3ButtonSlot = document.getElementById("selectMenu3-button-slot");
+    const selectMenu3Button0 = document.getElementById("selectMenu3-button0");
+
+    assert_false(selectMenu3.open);
+
+    let button1 = document.createElement("div");
+    button1.innerText = "button1";
+    button1.setAttribute("part", "button");
+    selectMenu3ButtonSlot.appendChild(button1);
+
+    await clickOn(button1);
+    assert_false(selectMenu3.open, "A button part should only get controller code if it's first in document order, even if added dynamically");
+
+    await clickOn(selectMenu3Button0);
+    assert_true(selectMenu3.open, "A button part should get controller code if it's first in document order");
+  }, "Button controller code should be applied in flat tree traversal order regardless of dynamic insertion order");
+
+  promise_test(async () => {
+    const selectMenu4 = document.getElementById("selectMenu4");
+    const selectMenu4Button0 = document.getElementById("selectMenu4-button0");
+    const selectMenu4ListboxSlot = document.getElementById("selectMenu4-listbox-slot");
+    const selectMenu4Option2 = document.getElementById("selectMenu4-option2");
+
+    assert_false(selectMenu4.open);
+
+    let listbox2 = document.createElement("div");
+    listbox2.innerHTML = `
+      <option>three</option>
+      <option id="selectMenu4-option4">four</option>
+    `;
+    listbox2.setAttribute("part", "listbox");
+    selectMenu4ListboxSlot.appendChild(listbox2);
+
+    await clickOn(selectMenu4Button0);
+    assert_true(selectMenu4.open);
+
+    const selectMenu4Option4 = document.getElementById("selectMenu4-option4");
+    await clickOn(selectMenu4Option4);
+    assert_equals(selectMenu3.value, "one", "An option in a listbox should not get controller code if its listbox isn't first in document order, even if added dynamically");
+
+    await clickOn(selectMenu4Button0);
+    assert_true(selectMenu4.open);
+
+    await clickOn(selectMenu4Option2);
+    assert_equals(selectMenu4.value, "two", "An option in a listbox should get controller code if its listbox is first in document order, even if another listbox was added dynamically");
+}, "Listbox controller code should be applied in flat tree traversal order regardless of dynamic insertion order");
 </script>
diff --git a/html/semantics/forms/the-selectmenu-element/selectmenu-popup.tentative.html b/html/semantics/forms/the-selectmenu-element/selectmenu-popup.tentative.html
@@ -29,6 +29,14 @@
   <!-- Swap out the listbox part without providing a replacement -->
   <div slot="listbox"></div>
 </selectmenu>
+
+<selectmenu id="selectMenu3">
+  <div slot="listbox">
+    <popup part="listbox" id="selectMenu3-listbox">
+      <option>one</option>
+    </popup>
+  </div>
+</selectmenu>
 <script>
 
   function clickOn(element) {
@@ -82,7 +90,19 @@
     const selectMenu2 = document.getElementById("selectMenu2");
     await clickOn(selectMenu2);
     assert_equals(selectMenu2.value, "");
+
+    // TODO(crbug.com/1234899) Fails because listbox pointer isn't cleared when default listbox is swapped out due to initial <slot> replacement
     assert_equals(selectMenu2.open, false);
 
   }, "Clicking a popup with no listbox part does nothing");
+
+  promise_test(async () => {
+    const selectMenu3 = document.getElementById("selectMenu3");
+    const selectMenu3Listbox = document.getElementById("selectMenu3-listbox");
+    selectMenu3Listbox.remove();
+
+    await clickOn(selectMenu3);
+    assert_equals(selectMenu3.value, "");
+    assert_equals(selectMenu3.open, false);
+  }, "Clicking a popup with a listbox that was removed does nothing");
 </script>