Skip to content

Commit b31a712

Browse files
mfreed7chromium-wpt-export-bot
authored andcommitted
Reland "Refactor menu event handling code [2/4]"
This is a reland of commit 95f912cedcfed49b1360d308897f441882b57f13 Original change's description: > Refactor menu event handling code [2/4] > > Another refactoring CL for menu element code. Previously, the command > invoker logic was split (and roughly duplicated) between > HTMLButtonElement and HTMLMenuItemElement. Since both can be command > invokers, it makes sense to move the command management stuff to the > common ancestor class, HTMLElement. This CL does that, and de-dupes > the code out of both subclasses. There is now just one base class > method that is overridden in sub-classes that can be command > invokers: CanBeCommandInvoker(). > > There was also a weird combination of popover activation command > code, which belongs on HTMLElement because any element can be a > popover, and menu activation command code, which belongs on the > HTMLMenuItemElement class, because only that class handles menu > commands. > > This touches a few outside classes like ax_object, which get simpler > now that they don't have to deal with each case of a command invoker. > > There is one behavior change here: now, if a menuitem is *both* > checkable *and* a sub-menu invoker, the sub-menu wins, and the > item does not do checked/unchecked behaviors. > > I also added a test that seemed missing: make sure only buttons and > menuitems can be command invokers. Even though <menuitem> isn't > shipped, this test should pass because it explicitly skips <menuitem>, > and anyway menuitem isn't included in the HTML5_ELEMENTS list. > > Bug: 406566432 > Change-Id: I281eda0eda4b0dced3ad38bd09a488f2c6b1ddf4 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7078858 > Reviewed-by: Dominic Farolino <dom@chromium.org> > Commit-Queue: Mason Freed <masonf@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1538871} Bug: 406566432 Change-Id: Iec91410726ae22a3673bdf32eb64aad79f672d6a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7113161 Commit-Queue: Dominic Farolino <dom@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/main@{#1540144}
1 parent be09b58 commit b31a712

File tree

2 files changed

+70
-5
lines changed

2 files changed

+70
-5
lines changed

html/semantics/menu/tentative/menubar-invoke-menulist.html

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,20 +128,33 @@
128128
}, "Menuitem with invalid command/commandfor cannot invoke menulist popover.");
129129

130130
test(() => {
131-
checkableMenuitem.command = "toggle-menu";
132-
checkableMenuitem.commandForElement = menulist;
133-
131+
assert_equals(checkableMenuitem.commandForElement,null,
132+
"To start, checkable item shouldn't be an invoker")
134133
checkableMenuitem.click();
135134
assert_true(checkableMenuitem.checked,
136135
"checkable menu item becomes checked");
137-
assert_true(menulist.matches(":popover-open"),
138-
"menulist matches :popover-open");
136+
assert_false(menulist.matches(":popover-open"),
137+
"not an invoker yet");
139138

140139
checkableMenuitem.click();
141140

142141
assert_false(checkableMenuitem.checked,
143142
"checkable menu item is no longer checked");
144143
assert_false(menulist.matches(":popover-open"),
145144
"menulist no longer matches :popover-open");
145+
146+
// Being an invoker for a sub-menu causes checkability to stop.
147+
checkableMenuitem.command = "toggle-menu";
148+
checkableMenuitem.commandForElement = menulist;
149+
checkableMenuitem.click();
150+
assert_false(checkableMenuitem.checked,
151+
"checkable menu item that invokes a menu does not become checked");
152+
assert_true(menulist.matches(":popover-open"),
153+
"menulist matches :popover-open");
154+
checkableMenuitem.click();
155+
assert_false(checkableMenuitem.checked,
156+
"checkable menu item is still not checked");
157+
assert_false(menulist.matches(":popover-open"),
158+
"menulist no longer matches :popover-open");
146159
}, "Checkable menuitems can still invoke menulist popovers");
147160
</script>
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<!doctype html>
2+
<meta name="timeout" content="long" />
3+
<meta charset="utf-8" />
4+
<meta name="author" href="mailto:masonf@chromium.org" />
5+
<link rel="help" href="https://open-ui.org/components/invokers.explainer/" />
6+
<script src="/resources/testharness.js"></script>
7+
<script src="/resources/testharnessreport.js"></script>
8+
<script src="/resources/testdriver.js"></script>
9+
<script src="/resources/testdriver-actions.js"></script>
10+
<script src="/resources/testdriver-vendor.js"></script>
11+
<script src="resources/invoker-utils.js"></script>
12+
<script src="/html/resources/common.js"></script>
13+
14+
<meta name=variant content=?command=--custom-event&half=first>
15+
<meta name=variant content=?command=--custom-event&half=second>
16+
<meta name=variant content=?command=show-popover&half=first>
17+
<meta name=variant content=?command=show-popover&half=second>
18+
<meta name=variant content=?command=show-modal&half=first>
19+
<meta name=variant content=?command=show-modal&half=second>
20+
21+
<div id="invokee"></div>
22+
23+
<script>
24+
// The command parameter is provided by variants, to
25+
// effectively split this (slow) test into multiple runs.
26+
const urlParams = new URLSearchParams(window.location.search);
27+
command = urlParams.get('command');
28+
half = urlParams.get('half');
29+
const firstHalf = half === 'first';
30+
assert_true(firstHalf || half === 'second');
31+
32+
const allowed = ['button','input','menuitem'];
33+
const allTags = HTML5_ELEMENTS.filter(el => (!allowed.includes(el)));
34+
const midpoint = Math.floor(allTags.length / 2);
35+
const tags = firstHalf ? allTags.slice(0,midpoint) : allTags.slice(midpoint);
36+
let gotEvent = false;
37+
invokee.addEventListener('command',() => (gotEvent = true));
38+
for(const tag of tags) {
39+
promise_test(async () => {
40+
gotEvent = false;
41+
const element = document.createElement(tag);
42+
element.setAttribute('commandfor','invokee');
43+
element.setAttribute('command',command);
44+
element.style.display = 'block'; // For normally invisible elements
45+
document.body.appendChild(element);
46+
// Click two ways
47+
element.click();
48+
await clickOn(element);
49+
assert_false(gotEvent,'Command should not be fired');
50+
},`command/commandfor on <${tag}> with command=${command} should not function`);
51+
}
52+
</script>

0 commit comments

Comments
 (0)