Skip to content

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

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}

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}
@chromium-wpt-export-bot chromium-wpt-export-bot marked this pull request as ready for review November 4, 2025 18:34
@wpt-pr-bot wpt-pr-bot added the html label Nov 4, 2025
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit b31a712 into master Nov 4, 2025
26 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-36c251d538 branch November 4, 2025 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants