Add commandfor & command attributes to HTMLButtonElement

[keithamus]

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.

• At least two implementers are interested (and none opposed):
  • Mozilla Supportive & prototyping
  • Webkit
  • Chromium supportive; actively prototyping.
• Tests are written and can be reviewed and commented upon at:
  • Mark invokers tests as non-tentative web-platform-tests/wpt#42664 (PR to make the tests non-tentative)
• Implementation bugs are filed:
  • Chromium Bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1490919
  • Chromium Patch: https://chromium-review.googlesource.com/c/chromium/src/+/4926471
  • Gecko Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1856430
  • Gecko Patch: https://phabricator.services.mozilla.com/D190449
  • WebKit Bug: https://bugs.webkit.org/show_bug.cgi?id=262840
  • Webkit Patch: Add invoketarget & invokeaction attributes WebKit/WebKit#18794
• MDN issue is filed: Add documentation for the "command" and "commandfor" attributes, and "command" event. mdn/content#30829
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

/dom.html ( diff )
/form-elements.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/interaction.html ( diff )
/interactive-elements.html ( diff )
/webappapis.html ( diff )

[scottaohara]

I realize this is also in the steps for getting the popover target element, but in both cases I'm wondering why its specified to return null if the node is in the disable state?

Doing so, at least with <button popovertarget=foo disabled> testing in chrome, results in that element not exposing whether the popover is in the expanded or collapsed state - as I'm assuming due to

"If node is disabled, then return null."

that state gets removed. That's unexpected, to have that state removed based on whether the button is disabled or not. And for invokertarget - if it is really is going to do more than just show/hide content - there are a lot of other states that should still be exposed, regarless of if the element is in the disabled state or not.

edit: I can file a bug for disabled / popovertarget if necessary - i just wanted to get insight on this first, before I went and made that issue. cc @mfreed7

[lukewarlow]

Fwiw HTML requires a positive standards position or for chrome LGTMs on an intent to ship to be considered supportive.

Saying that Mozilla have marked their position as positive so that's 1 implementor interested.

I do wonder how this requirement works for a feature such as this which will require multiple PRs to add to the spec?

[domenic]

a feature such as this which will require multiple PRs to add to the spec?

I'm confused why this feature is being done as multiple PRs; it makes review a good deal harder.

[keithamus]

If it makes it easier to review I’m happy to put more into one PR. I figured it would be worthwhile splitting it into the core vs each elements behaviour as I imagine there will be more to discuss with each elements behaviour.

[domenic]

Well, it'd make it easier for me, but I haven't signed up to review yet, so no need to make any changes until we get some more opinions :)

Edited to add: the reason it makes it more difficult is that I don't think we want to accept the feature piecemeal.

[lukewarlow]

As per openui/open-ui#900 (comment) this'll need updating to only fire the event when the action is custom (has a hypen) or is recognised and valid (correct action name on correct element).

TLDR is that this will allow us to add default actions in future without conflicting with user land code.

[lukewarlow]

In openui/open-ui#952 (comment) we resolved that "Invokers v1 will be popover and dialog invoking."

This should help keep this initial PR as small as possible while also avoid the issue of reviewing stuff piecemeal.

So #9875 can be merged into this along with dialog related changes.

[mfreed7]

Fwiw HTML requires a positive standards position or for chrome LGTMs on an intent to ship to be considered supportive.

Saying that Mozilla have marked their position as positive so that's 1 implementor interested.

Chromium is explicitly supportive of this proposal, so I believe it has two implementer support (including Mozilla).

Is this PR in a state that it can get a review? I'm happy to do so, if it'd help.

[keithamus]

I'd be happy to get reviews, I think this is in a good position for that.

[mfreed7]

I'd be happy to get reviews, I think this is in a good position for that.

Done - I added a first set of comments.

[annevk]

Some comments apply to the next line instead due to a second unforeseen force push.

[annevk]

I thought we landed on a single hyphen here?

[keithamus]

I thought it was double hyphen, that's certainly how I've been steering the prototype. I'm happy to move to single hyphen if that's preferred.

[annevk]

I remember this being discussed, I just don't remember what we decided on. I guess double is okay, but it seems rather weird as we're not bound by the same limitations as CSS for syntax.

[ryantownsend]

While not bound by CSS limitations, would it not still be better to be consistent with CSS custom properties?

I don't recall using single hyphen prefixes anywhere else in web development apart from vendor-specific CSS properties (i.e. -webkit-...).

Given custom commands are author-specific, not vendor-specific, it would make sense to me as a developer to be aligned with custom properties in terms of using a double hyphen prefix.

[annevk]

Maybe? But why require people to type --play-piano when -play-piano provides all we need? Anyway, I don't feel strongly and we can always make it less strict later.

[mfreed7]

We discussed in WHATNOT and there was ambivalence between single- and double-dash. We discussed in OpenUI and there was strong developer preference for consistency with CSS, with a double-dash.

[keithamus]

@annevk I re-worked the submit button piece a bit in d897bd8, as we discussed in the matrix channel. This commit removes the dl/dds that existed within the activation steps, instead opting for some ifs. Perhaps more impactful, however, is that it states that a button is only a "submit button" (concept) if it is implicitly or explicitly in the submit state but without command/commandfor attributes. The activation steps then branch if the element is a "submit button" (concept). Hopefully this is good and expresses what was discussed.

[annevk]

@keithamus I don't see how that fixes the <button> element type getter?

[keithamus]

@annevk I tried to expand the type setter steps to use the same kind of list as command, this way it'll only return "submit" if the button is a "submit button" - which it won't be if command/commandfor is set. This does mean the type property on <button type="submit" commandfor=""> will be 'button' - but this seems like it is desired?

[annevk]

<button type="submit" commandfor=""> I thought we would require both attributes?

[annevk]

This needs to be DOMString now, right? Without question mark.

[annevk]

Submit here should be lowercase. It's an algorithm, not a state. Right?

[annevk]

No need for a comma before and as there's only two conditions.

"then return". But also, isn't the first condition redundant with the second? Maybe this should be the first step, then a check if type attribute is in the reset state, and then returning button.