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

[Popup] popup="" is supported on <button type=submit> but not <input type=submit> #409

Closed
zcorpan opened this issue Oct 13, 2021 · 13 comments
Labels
popover The Popover API

Comments

@zcorpan
Copy link
Contributor

zcorpan commented Oct 13, 2021

https://open-ui.org/components/popup#the-popup-attribute

The popup attribute is supported on a subset of interactive elements:

button
input in the button state
input in the email, number, search, tel, text, or url states

There's an inconsistency between the first two: popup is supported on all button elements, including submit and reset buttons, but for input element only the Button state is supported.

https://html.spec.whatwg.org/multipage/form-elements.html#attr-button-type
https://html.spec.whatwg.org/multipage/input.html#attr-input-type

@gregwhitworth
Copy link
Member

@mfreed7 to weigh in

@gregwhitworth gregwhitworth added the popover The Popover API label Oct 20, 2021
@gregwhitworth
Copy link
Member

@mfreed7 friendly ping

@melanierichards
Copy link
Collaborator

This is my spec error. Personal POV is that we fix the discrepancy by limiting support to button in the button state, and do not support on reset or submit. The reset and submit states imply specific actions that will occur, and the button state is more appropriate to general-purpose commands (i.e. progressively show supporting UI).

@mfreed7
Copy link
Collaborator

mfreed7 commented Nov 4, 2021

Sorry for the delay, but I agree with @melanierichards. “Buttons” in either form (<button> or <input type=button>) should be supported. Buttons with prescribed actions (submit, reset) should not be supported. I think this makes sense and is consistent.

@melanierichards melanierichards added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Nov 4, 2021
@melanierichards
Copy link
Collaborator

Thanks Mason! Adding to the agenda, hopefully this will be a quick resolution.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed https://github.com/openui/open-ui/issues/409, and agreed to the following:

  • RESOLVED: <input type=button> should be supported for the popup attribute, except for submit, reset, and other buttons that would submit or reset a form.
The full IRC log of that discussion <melanierichards> topic: https://github.com//issues/409
<hdv> github: https://github.com//issues/409
<hdv> masonf: the popup attr lets you declaratively connect a popup to another element. it seems that should work from any kind of button, including a button, input type button or elements in a button state
<hdv> masonf: it should probably say buttons that aren't in the submit or reset state
<hdv> s/input type button/input type=button
<hdv> domenic: what about image buttons?
<hdv> masonf: seems to be that should work as well
<hdv> masonf: popups require some sort of activation behavior
<melanierichards> https://www.irccloud.com/pastebin/iIETDloL/
<hdv> melanierichards: i'll put the draft text in IRC
<hdv> domenic: seems more similar to input type=submit than input type=button
<hdv> domenic: as their default behavior is to submit the form given some coordinates
<melanierichards> q?
<hdv> masonf: then I should retract what I said before
<hdv> domenic: when you think of weird button edge cases, should think of image buttons
<hdv> melanierichards: masonf, do you want to write the resolution text?
<hdv> RRSAgent, draft minutes please
<RRSAgent> I have made the request to generate https://www.w3.org/2021/11/18-openui-minutes.html hdv
<masonf> proposed resolution: <input type=button> should be supported for the popup attribute, but submit, reset, and image buttons should not.
<melanierichards> Q?
<hdv> melanierichards: any concerns or disagreements?
<hdv> briankardell: domenic's comment made me curious… if a button element has an associated form, it would submit that too, right? does that affect this at all?
<hdv> masonf: I would lump that in with the submit button
<hdv> masonf: we would disallow using the popup attribute on a button whose behaviour is also to submit the form
<hdv> masonf: maybe we should add the input type=submit to the list too in the resolution
<hdv> scottohara: should it be explicit and only allowed on type=button
<melanierichards> q?
<masonf> <input type=button> should be supported for the popup attribute, but submit, reset, and image buttons should not, and neither should buttons with a form owner.
<hdv> domenic: that would work spec wise but maybe not as great to use
<hdv> melanierichards: agreed
<hdv> briankardell: is it right to say 'buttons with a form owner' ?
<hdv> domenic: or… reset buttons?
<masonf> proposed resolution: <input type=button> should be supported for the popup attribute, except for submit, reset, and other buttons that would submit a form.
<hdv> melanierichards how about image buttons?
<melanierichards> q?
<hdv> domenic: yes they always submit a form
<masonf> proposed resolution: <input type=button> should be supported for the popup attribute, except for submit, reset, and other buttons that would submit or reset a form.
<hdv> robflack: does this include the ARIA button role?
<hdv> domenic: no
<hdv> scottohara: no
<hdv> s/robflack/flackr
<melanierichards> RESOLVED: <input type=button> should be supported for the popup attribute, except for submit, reset, and other buttons that would submit or reset a form.
<hdv> resolved: <input type=button> should be supported for the popup attribute, except for submit, reset, and other buttons that would submit or reset a form.

@zcorpan
Copy link
Contributor Author

zcorpan commented Nov 18, 2021

From what I can tell from the meeting notes above, the input element was discussed, but not the different types of the button element. So that we're clear, is the resolution to allow popup for:

  • button element whose type attribute is in the Button state
  • input element whose type attribute is in the Button state
  • input element whose type attribute is in the Email, Number, Search, Telephone, Text, or URL states

@scottaohara
Copy link
Collaborator

The different types of buttons were discussed, but allowing popup on the different text inputs you mentioned was only briefly mentioned. It does seem reasonable that popup would be allowed on the inputs that presently allow the list attribute, so that authors can one day create styled type-ahead comboboxes.

per buttons specifically, the gist was that popup should not be allowed on a button that would submit or reset a form.

So for any of the following that are descendants of a form which can be submitted, or associated with such a form, popup would not be allowed:

  • input type=submit
  • input type=reset
  • input type=image
  • button with no type since it defaults to submit
  • button type=submit
  • button type=reset

But if any of those button types are used outside of a form / not associated with a form, in reality the reason to not use popop on these types goes away, since they will not submit or reset anything.

@zcorpan
Copy link
Contributor Author

zcorpan commented Nov 22, 2021

Making the rule depend on whether the button is associated with a form seems like unnecessary complication and confusion IMO. What's the benefit?

@scottaohara
Copy link
Collaborator

Understood. The point is that a button can't both invoke a popup and submit/reset a form though. So either the popup attribute is only allowed on buttons that have an explicit type=button, or the attribute is allowed on buttons so long as they don't submit a form.

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Dec 1, 2021
@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 8, 2022

To quickly summarize:

Implementation-wise, at least in Chromium, that'd be relatively easy to do within the default domactivate event handler for buttons (1, 2, 3) - we'd just check if the button being activated would be a successful form submitter, and if so, submit the form. Otherwise, activate the popup. How difficult this would be to spec, I don't know.

@scottaohara
Copy link
Collaborator

Would it be possible to have the toggle pop up attribute take priority over a button’s form submission functionality?

that would probably be helpful for situations where an author uses a button element and specifies the togglepopup attribute but fails to specify type=button

@mfreed7
Copy link
Collaborator

mfreed7 commented May 13, 2022

Would it be possible to have the toggle pop up attribute take priority over a button’s form submission functionality?

that would probably be helpful for situations where an author uses a button element and specifies the togglepopup attribute but fails to specify type=button

I'm inclined to think we should avoid touching form submission logic. For example, I could see confusion when an otherwise-working form that uses a type-less <button> to submit, and a developer adds togglepopup to that button. Now the button doesn't submit the form. I think I'd prefer, in that case, that the developer make a conscious choice to no longer submit the form, rather than the system silently doing that for them. (I've been bitten by form submission bugs too many times in the past, I think. 😄 )

Given the resolution and subsequent clarifications on this issue, I'd like to close it as resolved. If you disagree with my above thoughts, perhaps you could open a fresh issue to discuss?

@mfreed7 mfreed7 closed this as completed May 13, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 25, 2022
With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 26, 2022
With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
aarongable pushed a commit to chromium/chromium that referenced this issue May 26, 2022
With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656513
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007708}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 26, 2022
With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656513
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007708}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 26, 2022
With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656513
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007708}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 27, 2022
… buttons and text fields, a=testonly

Automatic update from web-platform-tests
Add popup support for <input>s styled as buttons and text fields

With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656513
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007708}

--

wpt-commits: 98feb29b0ae77dd554deeeb9715dabbac4891484
wpt-pr: 34200
jamienicol pushed a commit to jamienicol/gecko that referenced this issue May 30, 2022
… buttons and text fields, a=testonly

Automatic update from web-platform-tests
Add popup support for <input>s styled as buttons and text fields

With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656513
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007708}

--

wpt-commits: 98feb29b0ae77dd554deeeb9715dabbac4891484
wpt-pr: 34200
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
With this CL, several <input> element types, in addition to the existing
support for <button>, will be able to use the invoking attributes
(togglepopup, showpopup, and hidepopup) to invoke a popup. This CL
also adds enforcement of the resolution below, so that any button
that would otherwise submit a form cannot also/instead trigger a
popup.

openui/open-ui#409 (comment)

Bug: 1307772
Change-Id: I7eb5cd726bcacd26de5085871d7c3077c1f78baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656513
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007708}
NOKEYCHECK=True
GitOrigin-RevId: 1c609d3e37a137c3977aa660672890024f06b67c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests

6 participants