commit be16986e59d6f18dbf57f8d352beac2fd1b5af9e
parent 59196df365c74c3f6a368660616dfb9e8bc7c1a2
Author: Mason Freed <masonf@chromium.org>
Date: Wed, 3 Dec 2025 14:42:59 +0000
Bug 2003546 [wpt PR 56409] - More cleanup of menu elements code [6/4], a=testonly
Automatic update from web-platform-tests
More cleanup of menu elements code [6/4]
This primarily adds a condition to the mousedown-drag-mouseup code
that makes sure the mouseup `<menuitem>` is different from the
mousedown `<menuitem>`. To do that, the popover picker mousedown
location was changed to "info" which includes the element itself.
It turns out, after quite a bit of exploration, that the "ignore
next dom activate" code is the only way to do this correctly. I
tried to come up with logic that would work without modifying the
behavior of DOMActivate, but couldn't. Fundamentally:
- When doing mousedown-drag-mouseup, we need to activate the sub-
menu on mousedown.
- No matter what, when there's a mousedown and a mouseup, the
input system will always fire a `click` which triggers a
`DOMActivate`.
- The default command invoker behavior is to activate command
invokers from the `DOMActivate` event. That needs to be avoided
in this specific case.
I at least cleaned up the behavior to explicitly avoid *just*
the command invoker for menus, rather than all default event
behavior for `DOMActivate`.
Along the way, I also cleaned up a few other things, such as:
- Removed explicit code for keyboard events for " " and Enter.
- Made checkable and submenu-invoker explicitly exclusive. This is
more intuitive, I think.
- Made sub-menu special behavior only happen for menu-specific
commands. I.e. `show-menu` is now different from `show-popover`.
- Popovers triggered by menuitems will now close the containing
menulist. That also seems more intuitive, since a popover is
distinct from menus, and shouldn't create a nested structure.
E.g. if a menu item triggers a "more info" card, the menu that
triggered it shouldn't stay open - that feels weird.
Bug: 406566432
Change-Id: I836ce75b7cdc040b1743953bff1efcef22970e2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7119885
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1552763}
--
wpt-commits: b90da17d59dc73bfeaa436e5c032e6fdf483b949
wpt-pr: 56409
Diffstat:
3 files changed, 58 insertions(+), 34 deletions(-)
diff --git a/testing/web-platform/tests/html/semantics/menu/tentative/focus-menu-elements-arrowoperations.html b/testing/web-platform/tests/html/semantics/menu/tentative/focus-menu-elements-arrowoperations.html
@@ -7,7 +7,7 @@
<link rel=help href=https://open-ui.org/components/menu.explainer>
<menubar>
- <menuitem id=btn commandfor="list" command="toggle-popover">Open</menuitem>
+ <menuitem id=btn commandfor="list" command="toggle-menu">Open</menuitem>
</menubar>
<menulist id="list">
diff --git a/testing/web-platform/tests/html/semantics/menu/tentative/menubar-invoke-menulist.html b/testing/web-platform/tests/html/semantics/menu/tentative/menubar-invoke-menulist.html
@@ -7,7 +7,7 @@
<menubar>
<menuitem id="menubaritem">More commands</menuitem>
<menuitem>Command 2</menuitem>
- <menuitem>Command 3</menuitem>
+ <menuitem id="opencheckable" commandfor="checkable" command="show-menu">Open checkable menu</menuitem>
</menubar>
<menulist id="more">
@@ -15,7 +15,7 @@
<menuitem>Command 2</menuitem>
</menulist>
-<menulist>
+<menulist id="checkable">
<fieldset checkable>
<menuitem id="checkable-menuitem">Show menu</menuitem>
</fieldset>
@@ -130,6 +130,7 @@ test(() => {
test(() => {
assert_equals(checkableMenuitem.commandForElement,null,
"To start, checkable item shouldn't be an invoker")
+ opencheckable.click(); // Open the menu
checkableMenuitem.click();
assert_true(checkableMenuitem.checked,
"checkable menu item becomes checked");
@@ -143,15 +144,15 @@ test(() => {
assert_false(menulist.matches(":popover-open"),
"menulist no longer matches :popover-open");
- // Being checkable causes sub-menu functionality to stop.
+ // Being a sub-menu invoker makes the item non-checkable.
checkableMenuitem.command = "toggle-menu";
checkableMenuitem.commandForElement = menulist;
checkableMenuitem.click();
- assert_true(checkableMenuitem.checked,
- "checkable menu item that invokes a menu becomes checked");
- assert_false(menulist.matches(":popover-open"), "menulist is not open");
+ assert_false(checkableMenuitem.checked,
+ "checkable menu item that invokes a menu is not checkable");
+ assert_true(menulist.matches(":popover-open"), "menulist is open");
checkableMenuitem.click();
- assert_false(checkableMenuitem.checked, "checkable menu item unchecks");
- assert_false(menulist.matches(":popover-open"), "menulist still not open");
-}, "Checkable menuitems can still invoke menulist popovers");
+ assert_false(checkableMenuitem.checked, "still not checked");
+ assert_false(menulist.matches(":popover-open"), "menulist closes");
+}, "menuitems that invoke menulists cannot be checkable");
</script>
diff --git a/testing/web-platform/tests/html/semantics/menu/tentative/menuitem-activate.html b/testing/web-platform/tests/html/semantics/menu/tentative/menuitem-activate.html
@@ -72,11 +72,14 @@ promise_test(async () => {
assert_false(popover.matches(":popover-open"), "div popover starts closed");
await test_driver.click(mainmenuitem2);
assert_true(popover.matches(":popover-open"), "div popover opens");
+ assert_false(mainmenu.matches(":popover-open"), "mainmenu popover closes");
// Close the popover.
- await test_driver.click(mainmenuitem2);
+ await test_driver.click(menubarmenuitem);
assert_false(popover.matches(":popover-open"), "div popover gets closed");
- assert_true(mainmenu.matches(":popover-open"), "mainmenu still open");
+ assert_true(mainmenu.matches(":popover-open"), "mainmenu gets opened");
+ await test_driver.click(menubarmenuitem);
+ assert_false(mainmenu.matches(":popover-open"), "mainmenu gets closed");
}, 'User menuitem activation works with show-popover command');
promise_test(async (t) => {
@@ -121,51 +124,71 @@ promise_test(async (t) => {
}, 'Menulist inside a popover works correctly; does not get accidentally ' +
'dismissed by opening submenus');
+async function getCoords(invoker, invokee) {
+ // test_driver isn't suited to mousedown-drag-mouseup interactions when the
+ // mousedown triggers visibility of one of the elements.
+ await clickOn(invoker);
+ const menulist = invokee.parentElement;
+ assert_true(menulist.matches(":popover-open"), "menulist popover opens when clicked");
+ let rect = invokee.getBoundingClientRect();
+ let coords = {x: Math.round(rect.x + rect.width / 2),
+ y: Math.round(rect.y + rect.height / 2)};
+ await test_driver.click(invoker);
+ assert_false(menulist.matches(":popover-open"), "menulist popover closes when clicked");
+ return coords;
+}
+
promise_test(async (t) => {
assert_false(mainmenu.matches(":popover-open"), "mainmenu popover starts closed");
- let clickCount = 0;
- normalmenuitem.addEventListener('click',() => (++clickCount));
+ const normal_menu_coords = await getCoords(menubarmenuitem, normalmenuitem);
+ let invokerClicks = 0;
+ let itemClicks = 0;
+ menubarmenuitem.addEventListener('click',() => (++invokerClicks));
+ normalmenuitem.addEventListener('click',() => (++itemClicks));
+ let openStateAfterPointerdown = "none";
+ menubarmenuitem.addEventListener('pointermove',() => {
+ // There will be two move events, one before the pointerdown and one after.
+ // Just capture the one after.
+ if (openStateAfterPointerdown === "none") {
+ openStateAfterPointerdown = "first-move";
+ } else if (openStateAfterPointerdown === "first-move") {
+ openStateAfterPointerdown = mainmenu.matches(":popover-open") ? "open" : "closed";
+ }
+ },{signal: t.get_signal()});
await new test_driver.Actions()
.addPointer('mouse', 'mouse')
.pointerMove(0, 0, {origin: menubarmenuitem})
.pointerDown()
- .send();
- await waitForRender();
- assert_true(mainmenu.matches(":popover-open"), "mainmenu popover should be open while mouse is down");
- assert_equals(clickCount,0, "no clicks yet");
- await new test_driver.Actions()
- .addPointer('mouse', 'mouse')
- .pointerMove(0, 0, {origin: normalmenuitem})
+ // Extra move to trigger event on menubarmenuitem:
+ .pointerMove(2, 2, {origin: menubarmenuitem})
+ // This is the center of normalmenuitem:
+ .pointerMove(normal_menu_coords.x, normal_menu_coords.y, {})
.pointerUp()
.send();
await waitForRender();
- assert_false(mainmenu.matches(":popover-open"), "mainmenu popover should be closed");
- // TODO: Menu items should fire an event when they are selected.
- // The `click` event is not enough, because one won't be fired here.
- // assert_equals(clickCount,1, "the sub-menu item should have been clicked");
+ assert_equals(openStateAfterPointerdown,"open", "mainmenu popover should open after pointer down");
+ assert_false(mainmenu.matches(":popover-open"), "mainmenu popover should be closed after interaction");
+ assert_equals(invokerClicks,0, "the invoking menu didn't get a click");
+ // TODO: Menu items should fire a click event when they are selected.
+ // assert_equals(itemClicks,1, "the invoked menu did get a click");
}, 'A mousedown-drag-mouseup gesture on a normal menuitem picks the item');
promise_test(async (t) => {
assert_false(mainmenu.matches(":popover-open"), "mainmenu popover starts closed");
assert_false(submenu.matches(":popover-open"), "submenu popover starts closed");
+ const main_menu_coords = await getCoords(menubarmenuitem, mainmenuitem);
await new test_driver.Actions()
.addPointer('mouse', 'mouse')
.pointerMove(0, 0, {origin: menubarmenuitem})
.pointerDown()
- .send();
- await waitForRender();
- assert_true(mainmenu.matches(":popover-open"), "mainmenu popover should be open while mouse is down");
- assert_false(submenu.matches(":popover-open"), "submenu shouldn't be open yet");
- await new test_driver.Actions()
- .addPointer('mouse', 'mouse')
- .pointerMove(0, 0, {origin: mainmenuitem})
+ // This is the center of mainmenuitem:
+ .pointerMove(main_menu_coords.x, main_menu_coords.y, {})
.pointerUp()
.send();
await waitForRender();
assert_true(mainmenu.matches(":popover-open"), "mainmenu popover should remain open, because submenu chosen");
assert_true(submenu.matches(":popover-open"), "submenu popover should be open");
- menubarmenuitem.click(); // Cleanup.
- await waitForRender();
+ await clickOn(menubarmenuitem); // Cleanup.
assert_false(mainmenu.matches(":popover-open"), "mainmenu popover should be closed");
assert_false(submenu.matches(":popover-open"), "submenu popover should be closed");
}, 'A mousedown-drag-mouseup gesture on a submenu item leaves both menus open');
