Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Invoker Buttons - allowing popover/dialog and more to be invoked without JS #9625

Open
keithamus opened this issue Aug 23, 2023 · 141 comments
Labels
accessibility Affects accessibility addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: dialog The <dialog> element topic: events topic: forms topic: popover The popover attribute and friends

Comments

@keithamus
Copy link
Contributor

keithamus commented Aug 23, 2023

Following on from #3567 and #9456 where we tried to specify a way to open dialogs without JavaScript, @nt1m and @smaug---- raised concerns that the attribute was not extensible.

I've taken the feedback, and instead I'm proposing a new set of attributes that allow for opening dialogs and popovers, and also allow for extensibility for other interactions. I'll quote the summary:

Adding invokertarget and invokeraction attributes to <button> and
<input type="button"> / <input type="reset"> elements would allow
authors to assign behaviour to buttons in a more accessible and
declarative way, while reducing bugs and simplifying the amount of
JavaScript pages are required to ship for interactivity. Buttons with
invokertarget will - when clicked, touched, or enacted via keypress -
dispatch an InvokeEvent on the element referenced by invokertarget,
with some default behaviours.

In addition, adding an interesttarget attribute to
<button>, <a>, <input> elements would allow disclosure of high
fidelity tooltips in a more accessible and declaritive way. Elements
with interesttarget will - when hovered, long pressed, or focussed -
dispatch an InterestEvent on the element referenced by interesttarget,
with some default behaviours.

I'm soliciting feedback on this, and if we think this is more tenable than #9456 I'm happy to go forward with specs/implementations.

@keithamus keithamus added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest accessibility Affects accessibility topic: forms topic: events topic: dialog The <dialog> element topic: popover The popover attribute and friends labels Aug 23, 2023
@mfreed7
Copy link
Contributor

mfreed7 commented Aug 24, 2023

Nice proposal and explainer! I'm generally supportive of the direction this is going, and I like that you've really expanded the capabilities to include things other than <dialog> and popovers. I also see you've added an interesttarget mechanism via events to be able to handle both the "activate" and "hover" use cases, which is great.

A few small comments related to parts of the proposal:

In the style of popovertarget, this document proposes we add invokertarget, and invokeraction as available attributes to <button>, <input type="button"> and <input type="reset"> elements,

I think you'd want invokeraction on just the set of things we currently allow, which are those "buttons" you listed, but only when they don't participate in a form in some way (submitting or resetting). Mostly, I think that's what you meant, I just wanted to be clear.

as well as an interesttarget attribute to <button>, <a>, <input type="button"> and <input type="success"> elements.

Here, I think you likely can add back <input type=reset> and buttons that do participate in form submission, since you'd like to use this feature to provide context for those actions before you commit to them, and there's seemingly no conflict between this new "interest" feature and actually activating the elements. (Side note, I don't think <input type=success> is a thing.)

The invokertarget value should be an IDREF pointing to an element within the document. .invokerTargetElement also exists on the element to imperatively assign a node to be the invoker target, allowing for cross-root invokers.

I don't think this allows cross-root invokers, except in some cases. Note the complex conditions here.

If an element also has a popovertarget attribute then invokertarget must be ignored. interesttarget can exist on the element at the same time as popovertarget.

Thanks for considering what happens with both attributes present. I think it should likely go the other way though - if you have both, respect the new invokertarget and ignore popovertarget.

Loses/Lost Interest: The action of Loses Interest refers to the user "moving away" from an element...

It should be made clear that this applies to the pair of the invoker element and the target element. For example, a button that opens a popover - to lose interest while using a mouse, you'd have to de-hover both the button and the popover.


Overall, awesome proposal! I think it'd be a good idea to chat about this at an OpenUI meeting soon. I don't think I can Agenda+ it with any meaningful label, but would you mind if I get it on the next meeting's agenda?

@mfreed7
Copy link
Contributor

mfreed7 commented Aug 24, 2023

Also side note: see w3c/csswg-drafts#9236 for a very related CSS proposal to control the delays associated with gaining and losing "interest".

@scottaohara
Copy link
Collaborator

would this cover a button that might need to have an associated tooltip, but also opens a popover, popover dialog, or modal dialog?

@Westbrook
Copy link

How could the platform support having other elements (e.g. Custom Elements) to be invokers as well?

@keithamus
Copy link
Contributor Author

@mfreed7

I think you'd want invokeraction on just the set of things we currently allow, which are those "buttons" you listed, but only when they don't participate in a form in some way (submitting or resetting). Mostly, I think that's what you meant, I just wanted to be clear.

I'd like us to consider that <input type="reset"> could participate in a dialog form which would allow it to reset the form and close the dialog. I've updated the readme to reflect your comments though.

Here, I think you likely can add back <input type=reset> and buttons that do participate in form submission, since you'd like to use this feature to provide context for those actions before you commit to them, and there's seemingly no conflict between this new "interest" feature and actually activating the elements. (Side note, I don't think <input type=success> is a thing.)

I have no idea how input type=success came about. I guess ChatGPT isn't the only thing that can hallucinate prose 😆.

I don't think this allows cross-root invokers, except in some cases. Note the complex conditions here.

Thanks, added.

Thanks for considering what happens with both attributes present. I think it should likely go the other way though - if you have both, respect the new invokertarget and ignore popovertarget.

Yes that's a much better idea! Added.

It should be made clear that this applies to the pair of the invoker element and the target element. For example, a button that opens a popover - to lose interest while using a mouse, you'd have to de-hover both the button and the popover.

Great point! Added

Overall, awesome proposal! I think it'd be a good idea to chat about this at an OpenUI meeting soon. I don't think I can Agenda+ it with any meaningful label, but would you mind if I get it on the next meeting's agenda?

Please, this would be great to discuss further.

@scottaohara

would this cover a button that might need to have an associated tooltip, but also opens a popover, popover dialog, or modal dialog?

Yes I believe so:

<button interesttarget="my-tooltip" invokertarget="my-dialog">Tooltip on hover/focus, click to open dialog</button>

@Westbrook

How could the platform support having other elements (e.g. Custom Elements) to be invokers as well?

It's a good question, and one that seems to largely revolve around this big open question of "how can a custom element be a button". We could add something to .elementInternals() but that feels like it somewhat defeats the point of this being declarative. One solution without adding any new mechanics is to create an element that delegates focus to the button in its shadowroot, but that still requires imperative assignment if it wants to target a cross-root target.

This is all to say I don't have a good answer for this, and it might need addressing at a larger scope than this proposal.

@scottaohara
Copy link
Collaborator

@keithamus so that helps clarify a bit, but i'm still not sure what type of dialog is going to be invoked from that. a non-modal, a non-modal popover (in the top layer) or a modal dialog. I've looked at the InvokeEvent table from the explainer, and i'm seeing an expectation that something is a popover, or a modal dialog, but not a non-modal dialog (popover or not).

is the intent for interest largely for tooltips? or is there the possibility that other types of content could be shown/hidden if associated in that way? not for or against whatever answer is given, just want to understand the potential UX ramifications of a dialog or large block of content showing up on someone simply trying to tab through an interface. Understanding this would also result in any accessibility properties that would need to be exposed for the interesttarget attribute

Two other bits:

  1. i assume this proposal would potentially overlap/negate the need for Consider "toggle" (e.g., show/hide - expand/collapse) button or attribute  openui/open-ui#700 (if so, great!)
  2. i'd like to propose some updates to the accessibility section (specifically the implicit ARIA attributes). I assume it's ok to make a PR against your explainer, or would it make sense to talk that section out? Maybe it's better to talk it out at some point, rather than assume i understand the whys behind some of those choices?

@keithamus
Copy link
Contributor Author

so that helps clarify a bit, but i'm still not sure what type of dialog is going to be invoked from that.

I've ignored the concept of non-modal-non-popover dialogs, and so an invokertarget pointing to a <dialog> (with no popover) will call showModal(), and <dialog popover> will call showPopover(). I'm making the assumption that we'll one day resolve #9376 and <dialog>.show() will be deprecated.

is the intent for interest largely for tooltips? or is there the possibility that other types of content could be shown/hidden if associated in that way?

Largely for tooltips. I can't think of a compelling use case that would not be disruptive to folks for anything else, but I'm hoping if there are some, someone will come forward with them.

i assume this proposal would potentially overlap/negate the need for Consider toggle (expand/collapse) button or attribute openui/open-ui#700 (if so, great!)

I think so! It seems like spiritually these issues align. I want this proposal to effectively capture/replicate/explain any built-in interactive element, including <details>. I've not done extensive research into what else conceptually uses aria-expanded but if I've missed any that you think warrant adding please do submit a PR! The table in the proposal is an attempt at an exhaustive list but was mostly drawn up from memory so I'm sure I've missed stuff.

i'd like to propose some updates to the accessibility section (specifically the implicit ARIA attributes). I assume it's ok to make a PR against your explainer, or would it make sense to talk that section out? Maybe it's better to talk it out at some point, rather than assume i understand the whys behind some of those choices?

Absolutely please do! It's safe to assume that the whys behind my choices come from a place of ignorance and doing what I think is right without any hard research.

@scottaohara
Copy link
Collaborator

scottaohara commented Aug 24, 2023

thanks for all that, @keithamus

your response about the non-modal dialog makes sense now with that context - as it didn't initially click to me before that <* popover> the asterisks would handle the dialog popover use case. I had just read that table as "popovers" and "dialogs" as separate things. error on my part. Re: the linked issue, I've been thinking a lot about what Domenic mentioned in that thread - the use case for a non-modal dialog needing to not render in the top layer... and I can't shake the idea that there's something to that.

I think so! It seems like spiritually these issues align

that was my read as well, which is great. I'll definitely come back and kick the tires on this some more, especially re: the next bit about updating some of the accessibility bits. Which truly thanks for even considering that stuff. It's mostly nits / suggestions to either match reality, or use this as an opportunity to force support / update the spec for a certain attribute.

i'll put this on my todo to do another review / make a PR. Thanks!

@mfreed7
Copy link
Contributor

mfreed7 commented Aug 31, 2023

is the intent for interest largely for tooltips? or is there the possibility that other types of content could be shown/hidden if associated in that way?

Largely for tooltips. I can't think of a compelling use case that would not be disruptive to folks for anything else, but I'm hoping if there are some, someone will come forward with them.

One such use case is a nested menu. (Try Google Docs for this exact behavior.) Click to open a menu (e.g. File), then hover over one of the items with a sub-menu. The sub-menu shows up after a slight delay. Note that their keyboard behavior is different - focusing on the item does nothing, and you have to hit the right-arrow to open the sub-menu.

@mfreed7
Copy link
Contributor

mfreed7 commented Aug 31, 2023

Note that their keyboard behavior is different - focusing on the item does nothing, and you have to hit the right-arrow to open the sub-menu.

I suppose if we go with interesttarget that means hover, focus, or long-press, and someone wanted the above behavior (only trigger on focus, because they've handled the other modes themselves) then they'd need to do something like preventDefault() on the appropriate events? That's a bit of an issue now because focus is not cancelable. Is there another suggestion about how to give interesttarget more configurability?

@scottaohara
Copy link
Collaborator

scottaohara commented Aug 31, 2023

thanks @mfreed7 - yes that's a good example, and a good reason why hint would have been a weird name for this type of popover. but yeh, per that google file menu example, it seems to me that hover is more of an 'enhancement' to that menuitem, as really someone should be able to tap/press on that using a touch device to open, since 'hover' and 'focus' may not even be things one can do if using a touch device, for example.

@mfreed7
Copy link
Contributor

mfreed7 commented Aug 31, 2023

So as mentioned above, we discussed this today at OpenUI, and the general tone was that this new proposal is generally a good direction to try to go. It has the flexibility needed to support many different element types, and it's expressive enough to work for many common use cases.

One point was made that the interesttarget part of the proposal still has some open questions, e.g. the particulars of the long-press behavior. It might be worthwhile splitting the proposal into two pieces, one for invokertarget which seems implementable and standardizable today, and one for interesttarget which needs more research on a few things.

What do folks think about this direction? @annevk @ntim @emilio

@nt1m
Copy link
Member

nt1m commented Aug 31, 2023

General idea seems like the right way to go, though there's probably some bikeshedding to be done around naming and smaller details.

@mfreed7
Copy link
Contributor

mfreed7 commented Sep 1, 2023

General idea seems like the right way to go, though there's probably some bikeshedding to be done around naming and smaller details.

Great! Do you think it's close enough that someone (@keithamus ??) should start drafting a spec PR? We're happy to prototype for Chromium. I do think we should just tackle invokertarget and not (yet) interesttarget to simplify things at first. Alternatively, two PRs could be drafted, one for each, and we could debate the issues around interesttarget via the PR?

My points of bikeshedding:

  • invokeraction=opendialog should just be invokeraction=showModal. As much as possible, the action names should match up with their JS counterparts.
  • Similarly, I'd prefer invokeraction=close for closing dialogs, invokeraction=open and close for opening and closing dialogs, invokeraction=pause and play for videos. The action doesn't need to include the context (e.g. play instead of playVideo), and it's ok for multiple types of targets to support the same action name, like open.

@keithamus
Copy link
Contributor Author

Happy to work on specs and implementations.

I think your bike shedding points make sense and I agree with them; I was just being conservative in the explainer 😜

@josepharhar
Copy link
Contributor

I'm not sure if these have already been discussed or not, but these feature requests for popovertarget should also be considered for this new attribute:

@mfreed7 mfreed7 added the agenda+ To be discussed at a triage meeting label Sep 7, 2023
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jul 16, 2024
…et->commandfor, a=testonly

Automatic update from web-platform-tests
Rename invokeaction->command, invoketarget->commandfor

This also drops the "auto" value, and updates all tests to accommodate
these changes, as well as adding more tests for extra robustness.

This is the result of a discussion around renaming the attribute which starts around whatwg/html#9625 (comment) and concludes in whatwg/html#9625 (comment).

Bug: 349994204
Change-Id: Ic4a5572506e855036c8c1f75aa0de894de026948
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5666705
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Keith Cirkel <chromium@keithcirkel.co.uk>
Cr-Commit-Position: refs/heads/main@{#1325205}

--

wpt-commits: 290314c241de3c980fc4eeeb3d19d3fada080b0e
wpt-pr: 47064
@annevk
Copy link
Member

annevk commented Jul 16, 2024

I would also like to discuss the CommandEvent members. I'd prefer not exposing invoker as a term. I don't see why the earlier arguments against that don't apply here as well.

@lukewarlow
Copy link
Member

We should discuss this openui/open-ui#1072 too. It's come up before but Tag never said anything during review.

@annevk
Copy link
Member

annevk commented Jul 17, 2024

Most enumerated attribute values (which are not in that table) indeed seem to use hyphens, e.g., crossorigin=use-credentials or referrer=no-referrer. scope=rowgroup is a notable exception (and the only one I could find). That does not seem like something that needs discussion, it just needs to be fixed.

@lukewarlow
Copy link
Member

I think the discussion is what should the requirement for a custom command name be? Currently we use "contains hyphen" which wouldn't work here.

@jakearchibald
Copy link
Contributor

CSS uses -- at the start, but the only reason it's two is because vendor prefixes 'claimed' the single -.

@domenic
Copy link
Member

domenic commented Jul 18, 2024

I wasn't able to find in the explainer why custom commands are supported at all. I thought the point of this feature was to help with no-JavaScript cases. But those cases require JavaScript to work. They're just adding some extra steps before the JavaScript, which in the end don't save much effort.

Would it be possible to omit them, at least from v1?

i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jul 18, 2024
…et->commandfor, a=testonly

Automatic update from web-platform-tests
Rename invokeaction->command, invoketarget->commandfor

This also drops the "auto" value, and updates all tests to accommodate
these changes, as well as adding more tests for extra robustness.

This is the result of a discussion around renaming the attribute which starts around whatwg/html#9625 (comment) and concludes in whatwg/html#9625 (comment).

Bug: 349994204
Change-Id: Ic4a5572506e855036c8c1f75aa0de894de026948
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5666705
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Keith Cirkel <chromium@keithcirkel.co.uk>
Cr-Commit-Position: refs/heads/main@{#1325205}

--

wpt-commits: 290314c241de3c980fc4eeeb3d19d3fada080b0e
wpt-pr: 47064
@ryantownsend
Copy link

@domenic from an author’s point of view, allowing custom events helps remove a significant volume of boilerplate JavaScript where click events need to be added/removed to trigger events on other elements. I’ve seen this ‘send this custom command to this element targeted by ID’ HTML pattern in pretty much every HTML-led JS library (Bootstrap, HTMX etc) since the 2000s, and I’ve little doubt much of the underpinnings of the larger libraries/frameworks (React, Vue, Angular etc) could either be replaced behind the scenes or removed altogether thanks to this native API.

Secondly, depending on what’s permitted, it could/should provide a route to polyfilling additional native commands (e.g. stepUp).

Thirdly, just like the native events, moving away from addEventListener and using declarative custom commands in the HTML means it’s far more clear what behaviour exists - better for maintainability of projects.

Finally, having custom events standardised in the platform means third party web components can adhere to this and listen on some custom events and developers using those components need nothing more than dropping a few HTML attributes in their <button> elements, making implementation easier, less error prone, less prone to first party performance issues etc.

sadym-chromium pushed a commit to web-platform-tests/wpt that referenced this issue Jul 18, 2024
This also drops the "auto" value, and updates all tests to accommodate
these changes, as well as adding more tests for extra robustness.

This is the result of a discussion around renaming the attribute which starts around whatwg/html#9625 (comment) and concludes in whatwg/html#9625 (comment).

Bug: 349994204
Change-Id: Ic4a5572506e855036c8c1f75aa0de894de026948
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5666705
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Keith Cirkel <chromium@keithcirkel.co.uk>
Cr-Commit-Position: refs/heads/main@{#1325205}
@past past removed the agenda+ To be discussed at a triage meeting label Jul 18, 2024
keithamus added a commit to keithamus/WebKit that referenced this issue Jul 24, 2024
https://bugs.webkit.org/show_bug.cgi?id=276616

Reviewed by NOBODY (OOPS!).

This renames the attributes invoketarget->commandfor and invokeaction->command,
as well as renaming the InvokeEvent to CommandEvent, and it's action property
to command. It also drops the `auto` action, requiring explicit
`command` attribute for each action.

Lastly, this also adds the logic preventing invokers from being triggered within
a form without an explicit type=button attribute.

These changes are based on a set of discussions around the proposals
naming and semantics, which starts here: whatwg/html#9625 (comment)
and concludes here; whatwg/html#9625 (comment)

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/idlharness.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeelement-interface.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeevent-dispatch-shadow.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeevent-interface.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-button-event-dispatch.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-popover-behavior.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-popover-invalid-behavior.tentative-expected.txt:
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Headers.cmake:
* Source/WebCore/SmartPointerExpectations/UncountedLocalVarsCheckerExpectations:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
* Source/WebCore/dom/CommandEvent.cpp: Renamed from Source/WebCore/dom/InvokeEvent.cpp.
(WebCore::CommandEvent::CommandEvent):
(WebCore::CommandEvent::create):
(WebCore::CommandEvent::createForBindings):
(WebCore::CommandEvent::isCommandEvent const):
(WebCore::CommandEvent::invoker const):
* Source/WebCore/dom/CommandEvent.h: Renamed from Source/WebCore/dom/InvokeEvent.h.
* Source/WebCore/dom/CommandEvent.idl: Renamed from Source/WebCore/dom/InvokeEvent.idl.
* Source/WebCore/dom/DeviceOrientationAndMotionAccessController.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::handlePopoverLightDismiss):
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::isElementReflectionAttribute):
* Source/WebCore/dom/Element.h:
(WebCore::Element::isValidCommandType):
(WebCore::Element::handleCommandInternal):
(WebCore::Element::isValidInvokeAction): Deleted.
(WebCore::Element::handleInvokeInternal): Deleted.
* Source/WebCore/dom/Event.h:
(WebCore::Event::isCommandEvent const):
(WebCore::Event::isInputEvent const):
(WebCore::Event::isInvokeEvent const): Deleted.
* Source/WebCore/dom/EventInterfaces.in:
* Source/WebCore/dom/EventNames.json:
* Source/WebCore/html/HTMLAttributeNames.in:
* Source/WebCore/html/HTMLButtonElement.cpp:
(WebCore::HTMLButtonElement::defaultEventHandler):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::isValidCommandType):
(WebCore::HTMLDialogElement::handleCommandInternal):
(WebCore::HTMLDialogElement::isValidInvokeAction): Deleted.
(WebCore::HTMLDialogElement::handleInvokeInternal): Deleted.
* Source/WebCore/html/HTMLDialogElement.h:
* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::isValidCommandType):
(WebCore::HTMLElement::handleCommandInternal):
(WebCore::HTMLElement::isValidInvokeAction): Deleted.
(WebCore::HTMLElement::handleInvokeInternal): Deleted.
* Source/WebCore/html/HTMLElement.h:
* Source/WebCore/html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::commandForElement const):
(WebCore::HTMLFormControlElement::commandType const):
(WebCore::HTMLFormControlElement::handleCommand):
(WebCore::HTMLFormControlElement::invokeTargetElement const): Deleted.
(WebCore::HTMLFormControlElement::invokeAction const): Deleted.
(WebCore::HTMLFormControlElement::handleInvokeAction): Deleted.
* Source/WebCore/html/HTMLFormControlElement.h:
* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::defaultEventHandler):
* Source/WebCore/html/InvokerElement.idl:
* Source/WebCore/page/FragmentDirective.h:
keithamus added a commit to keithamus/WebKit that referenced this issue Jul 25, 2024
https://bugs.webkit.org/show_bug.cgi?id=276616

Reviewed by NOBODY (OOPS!).

This renames the attributes invoketarget->commandfor and invokeaction->command,
as well as renaming the InvokeEvent to CommandEvent, and it's action property
to command. It also drops the `auto` action, requiring explicit
`command` attribute for each action.

Lastly, this also adds the logic preventing invokers from being triggered within
a form without an explicit type=button attribute.

These changes are based on a set of discussions around the proposals
naming and semantics, which starts here: whatwg/html#9625 (comment)
and concludes here; whatwg/html#9625 (comment)

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/idlharness.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeelement-interface.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeevent-dispatch-shadow.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeevent-interface.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-button-event-dispatch.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-popover-behavior.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-popover-invalid-behavior.tentative-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt: Copied from LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt.
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Headers.cmake:
* Source/WebCore/SmartPointerExpectations/UncountedLocalVarsCheckerExpectations:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
* Source/WebCore/dom/CommandEvent.cpp: Renamed from Source/WebCore/dom/InvokeEvent.cpp.
(WebCore::CommandEvent::CommandEvent):
(WebCore::CommandEvent::create):
(WebCore::CommandEvent::createForBindings):
(WebCore::CommandEvent::isCommandEvent const):
(WebCore::CommandEvent::invoker const):
* Source/WebCore/dom/CommandEvent.h: Renamed from Source/WebCore/dom/InvokeEvent.h.
* Source/WebCore/dom/CommandEvent.idl: Renamed from Source/WebCore/dom/InvokeEvent.idl.
* Source/WebCore/dom/DeviceOrientationAndMotionAccessController.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::handlePopoverLightDismiss):
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::isElementReflectionAttribute):
* Source/WebCore/dom/Element.h:
(WebCore::Element::isValidCommandType):
(WebCore::Element::handleCommandInternal):
(WebCore::Element::isValidInvokeAction): Deleted.
(WebCore::Element::handleInvokeInternal): Deleted.
* Source/WebCore/dom/Event.h:
(WebCore::Event::isCommandEvent const):
(WebCore::Event::isInputEvent const):
(WebCore::Event::isInvokeEvent const): Deleted.
* Source/WebCore/dom/EventInterfaces.in:
* Source/WebCore/dom/EventNames.json:
* Source/WebCore/html/HTMLAttributeNames.in:
* Source/WebCore/html/HTMLButtonElement.cpp:
(WebCore::HTMLButtonElement::defaultEventHandler):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::isValidCommandType):
(WebCore::HTMLDialogElement::handleCommandInternal):
(WebCore::HTMLDialogElement::isValidInvokeAction): Deleted.
(WebCore::HTMLDialogElement::handleInvokeInternal): Deleted.
* Source/WebCore/html/HTMLDialogElement.h:
* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::isValidCommandType):
(WebCore::HTMLElement::handleCommandInternal):
(WebCore::HTMLElement::isValidInvokeAction): Deleted.
(WebCore::HTMLElement::handleInvokeInternal): Deleted.
* Source/WebCore/html/HTMLElement.h:
* Source/WebCore/html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::commandForElement const):
(WebCore::HTMLFormControlElement::commandType const):
(WebCore::HTMLFormControlElement::handleCommand):
(WebCore::HTMLFormControlElement::invokeTargetElement const): Deleted.
(WebCore::HTMLFormControlElement::invokeAction const): Deleted.
(WebCore::HTMLFormControlElement::handleInvokeAction): Deleted.
* Source/WebCore/html/HTMLFormControlElement.h:
* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::defaultEventHandler):
* Source/WebCore/html/InvokerElement.idl:
* Source/WebCore/page/FragmentDirective.h:
webkit-commit-queue pushed a commit to keithamus/WebKit that referenced this issue Jul 25, 2024
https://bugs.webkit.org/show_bug.cgi?id=276616

Reviewed by Anne van Kesteren.

This renames the attributes invoketarget->commandfor and invokeaction->command,
as well as renaming the InvokeEvent to CommandEvent, and it's action property
to command. It also drops the `auto` action, requiring explicit
`command` attribute for each action.

Lastly, this also adds the logic preventing invokers from being triggered within
a form without an explicit type=button attribute.

These changes are based on a set of discussions around the proposals
naming and semantics, which starts here: whatwg/html#9625 (comment)
and concludes here; whatwg/html#9625 (comment)

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/idlharness.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeelement-interface.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeevent-dispatch-shadow.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invokeevent-interface.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-button-event-dispatch.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-popover-behavior.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-popover-invalid-behavior.tentative-expected.txt:
* LayoutTests/platform/ios/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt: Copied from LayoutTests/imported/w3c/web-platform-tests/html/semantics/invokers/invoketarget-on-dialog-behavior.tentative-expected.txt.
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-input.xcfilelist:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Headers.cmake:
* Source/WebCore/SmartPointerExpectations/UncountedLocalVarsCheckerExpectations:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
* Source/WebCore/dom/CommandEvent.cpp: Renamed from Source/WebCore/dom/InvokeEvent.cpp.
(WebCore::CommandEvent::CommandEvent):
(WebCore::CommandEvent::create):
(WebCore::CommandEvent::createForBindings):
(WebCore::CommandEvent::isCommandEvent const):
(WebCore::CommandEvent::invoker const):
* Source/WebCore/dom/CommandEvent.h: Renamed from Source/WebCore/dom/InvokeEvent.h.
* Source/WebCore/dom/CommandEvent.idl: Renamed from Source/WebCore/dom/InvokeEvent.idl.
* Source/WebCore/dom/DeviceOrientationAndMotionAccessController.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::handlePopoverLightDismiss):
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::isElementReflectionAttribute):
* Source/WebCore/dom/Element.h:
(WebCore::Element::isValidCommandType):
(WebCore::Element::handleCommandInternal):
(WebCore::Element::isValidInvokeAction): Deleted.
(WebCore::Element::handleInvokeInternal): Deleted.
* Source/WebCore/dom/Event.h:
(WebCore::Event::isCommandEvent const):
(WebCore::Event::isInputEvent const):
(WebCore::Event::isInvokeEvent const): Deleted.
* Source/WebCore/dom/EventInterfaces.in:
* Source/WebCore/dom/EventNames.json:
* Source/WebCore/html/HTMLAttributeNames.in:
* Source/WebCore/html/HTMLButtonElement.cpp:
(WebCore::HTMLButtonElement::defaultEventHandler):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::isValidCommandType):
(WebCore::HTMLDialogElement::handleCommandInternal):
(WebCore::HTMLDialogElement::isValidInvokeAction): Deleted.
(WebCore::HTMLDialogElement::handleInvokeInternal): Deleted.
* Source/WebCore/html/HTMLDialogElement.h:
* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::isValidCommandType):
(WebCore::HTMLElement::handleCommandInternal):
(WebCore::HTMLElement::isValidInvokeAction): Deleted.
(WebCore::HTMLElement::handleInvokeInternal): Deleted.
* Source/WebCore/html/HTMLElement.h:
* Source/WebCore/html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::commandForElement const):
(WebCore::HTMLFormControlElement::commandType const):
(WebCore::HTMLFormControlElement::handleCommand):
(WebCore::HTMLFormControlElement::invokeTargetElement const): Deleted.
(WebCore::HTMLFormControlElement::invokeAction const): Deleted.
(WebCore::HTMLFormControlElement::handleInvokeAction): Deleted.
* Source/WebCore/html/HTMLFormControlElement.h:
* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::defaultEventHandler):
* Source/WebCore/html/InvokerElement.idl:
* Source/WebCore/page/FragmentDirective.h:

Canonical link: https://commits.webkit.org/281345@main
@mfreed7
Copy link
Contributor

mfreed7 commented Jul 25, 2024

This issue/PR was just discussed at the OpenUI/WHATWG/CSSWG form controls task force meeting, in the context of a more general discussion of the "tooltip" use case. The notes are located here:

https://logs.csswg.org/irc.w3.org/css/2024-07-25/#e1637392

@annevk
Copy link
Member

annevk commented Aug 15, 2024

@keithamus you mentioned that the PR is up-to-date, but I don't think the PR incorporates restricting the feature to the button element. From the July 18 minutes:

luke: joey do you have feedback?
joey: yes only support the button element
olli: i still want this to be supported everywhere, but that's something we can figure out later
keith: yeah i guess it's easier to support button now and add it to additional elements if we feel that there's a strong use case for that
luke: sounds good to me
keith: sounds like we're all resolved then

I did not check if other decisions are incorporated, but it would be good to double check all of that.

@keithamus
Copy link
Contributor Author

@annevk doesn't 6767e99 incorporate that change? Or do I need to add some extra guards in the button activation routine?

@annevk
Copy link
Member

annevk commented Aug 16, 2024

@keithamus well in the index the attributes are listed as applying to both input and button for instance. The attributes are also still added to the input element table. It also seems like CommandElement can just be inlined into the button element at this point.

@zcorpan
Copy link
Member

zcorpan commented Sep 2, 2024

@keithamus (#9625 (comment))

This was discussed again at WHATNOT, mostly around the idea of having commandfor work inside of <form>s. We settled on exploring the following ideas to mitigate the backcompat concerns:

  • Form buttons with the commandfor attribute cannot be submit buttons. As such they will no longer submit the form (this is an older browser compat risk, however...).

    • In this way, a <button type=submit commandfor=..> inside a <form. is an author error and does nothing.
  • Form buttons with the commandfor attribute and an implicit type will also not handle the command. The commandfor steps will require a button to be an explicit type=button if they have an associated form.

    • In this way <button commandfor=..> inside a form will mean that the <button> is no longer a submit button, but also will not issue the command. Effectively the button becomes a no-op. Instead, user agents MAY issue a warning telling authors to either add type=button, or remove the commandfor= attribute.

This doesn't make sense to me. It introduces a behavior difference between a browser implementing commandfor vs those that do not. IMO it should still submit the form. I suggest instead to only ignore the command/commandfor attributes if the type is not in the expected state.

  • A <button commandfor=..> outside a <form> will work as expected.

type is in the "Submit" state here. Why not require the "Button" state always? It seems more robust and less confusing to have the same behavior in a form as outside of a form.

  • A <button type=button commandfor=..> inside a <form> will work as expected.

  • One day we may lift the restriction for type=button, such that <button commandfor=..> inside a form will invoke the command, however that will be a later step that may happen several years after shipping invokers, and we will need to assess the compact risk then. This step allows us to scope the compat risk.

Related; #10462 has been raised to explore the idea of removing the invalid default of submit for buttons.

@annevk
Copy link
Member

annevk commented Sep 3, 2024

@zcorpan we want to eventually change the default of the type attribute when the command and commandfor attributes are specified. These are small steps needed to nudge in that general direction. Ignoring the attributes would not be a step in the right direction that would allow us to change the default five-ten years from now.

keithamus added a commit to keithamus/html that referenced this issue Nov 8, 2024
This adds the commandfor & command attributes and a "command" event using the CommandEvent interface.

Button activation checks if the button has a "commandfor" target and if so performs invoker command
behaviour depending on command and the target element.

This introduces 1 new IDLs:

- CommandEvent: A new event that has `action` and `source`.

And extends the HTMLButtonElement IDL to introduce `command` and
`commandfor`

Things NOT covered in this commit that are included in the proposal (whatwg#9625):

- Default per element behaviours for anything beyond popover & dialog,
  this will be dealt with in subsequent individual commits.

- `interestaction` and `interesttarget`.
keithamus added a commit to keithamus/html that referenced this issue Nov 8, 2024
This adds the commandfor & command attributes and a "command" event using the CommandEvent interface.

Button activation checks if the button has a "commandfor" target and if so performs invoker command
behaviour depending on command and the target element.

This introduces 1 new IDLs:

- CommandEvent: A new event that has `action` and `source`.

And extends the HTMLButtonElement IDL to introduce `command` and
`commandfor`

Things NOT covered in this commit that are included in the proposal (whatwg#9625):

- Default per element behaviours for anything beyond popover & dialog,
  this will be dealt with in subsequent individual commits.

- `interestaction` and `interesttarget`.
@annevk
Copy link
Member

annevk commented Nov 8, 2024

To elaborate on the above comment, changing the default was discussed in #10441. This means that when the type attribute is omitted and the command and commandfor attributes are set, the type getter is to return "button". However, for now what happens when there's a form owner is that it no-ops to encourage people to set the type attribute explicitly to button for compatibility with older user agents where it would submit the form otherwise. In the future we can remove that explicit type attribute requirement when there is a form owner.

When the type attribute is set to something besides button and the command and commandfor attributes are set we should probably make the command and commandfor attributes ignored, though this was not explicitly discussed.

keithamus added a commit to keithamus/html that referenced this issue Nov 25, 2024
This adds the commandfor & command attributes and a "command" event using the CommandEvent interface.

Button activation checks if the button has a "commandfor" target and if so performs invoker command
behaviour depending on command and the target element.

This introduces 1 new IDLs:

- CommandEvent: A new event that has `action` and `source`.

And extends the HTMLButtonElement IDL to introduce `command` and
`commandfor`

Things NOT covered in this commit that are included in the proposal (whatwg#9625):

- Default per element behaviours for anything beyond popover & dialog,
  this will be dealt with in subsequent individual commits.

- `interestaction` and `interesttarget`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Affects accessibility addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: dialog The <dialog> element topic: events topic: forms topic: popover The popover attribute and friends
Development

No branches or pull requests