Add popover attribute #8221

josepharhar · 2022-08-24T20:48:00Z

Feature proposal issue: #7785
Explainer: https://open-ui.org/components/popup.research.explainer

At least two implementers are interested (and none opposed):
- Chrome
- Firefox: The Popover API mozilla/standards-positions#698
- Safari: The Popover API WebKit/standards-positions#74
Tests are written and can be reviewed and commented upon at:
- https://wpt.fyi/results/html/semantics/popups
Implementation bugs are filed:
- Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1307772
- Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1808823
- Safari: https://bugs.webkit.org/show_bug.cgi?id=250171

/browsers.html ( diff )
/dnd.html ( diff )
/dom.html ( diff )
/form-elements.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/input.html ( diff )
/interaction.html ( diff )
/interactive-elements.html ( diff )
/rendering.html ( diff )
/semantics-other.html ( diff )
/webappapis.html ( diff )
/popover.html ( diff )

josepharhar · 2022-08-24T20:54:17Z

I see that there are many build errors after build.sh says "Running conformance checker...", but when I run build.sh locally, it just stops after "Success!".

How do I run the conformance checker when building locally?

domenic · 2022-08-25T01:39:17Z

The conformance checker is at https://github.com/validator/validator and has a variety of installation options. You need to run it on the output index.html file.

josepharhar · 2022-08-25T20:49:37Z

The conformance checker is at https://githu 8000 b.com/validator/validator and has a variety of installation options. You need to run it on the output index.html file.

Thanks, I was able to run it! It looks like the PR builds now

source

josepharhar · 2022-09-06T21:53:01Z

@domenic I added some examples in 81acbc7 and 9e0421c. Can you point me to any examples of "descriptive text for web developers and for implementers" that you mentioned earlier?

domenic

I managed to get a start on this review today... still more to go, but hopefully this helps.

source

josepharhar · 2022-09-08T16:50:59Z

Thanks for the review! I'm going to be out until monday so I'll try to get to it then

source

josepharhar · 2022-09-14T00:27:01Z

Thanks for the review @domenic! I have addressed all your comments

Kaiido

Not an authoritative review; but some questions that popped up while quickly reading the PR.

source

Since these attributes only apply to HTMLButtonElement and HTMLInputElement, it doesn't make sense to put them in the IDL for all Elements: whatwg/html#8221 (comment) Change-Id: I7038aefedbff78ec3af97b23c7a51a64cb0f275d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3894822 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Nate Fischer <ntfschr@chromium.org> Reviewed-by: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1048107}

josepharhar · 2022-09-26T06:33:30Z

@domenic mind taking another look? Anything else I can do to help move this forward?

domenic · 2022-09-26T07:05:17Z

It's on my list, sorry! I had a week of TPAC then a week of vacation, so I'm currently down to 20 flagged work emails and 24 flagged GitHub emails :).

annevk · 2022-10-04T16:51:00Z

Drive-by: element and attribute indices haven't been updated as far as I can tell. (FWIW, I plan on doing a more careful review.)

domenic

Lots of editorial issues; as always, if you can be sure to do an extra sweep to make sure you catch multiple instances, that saves a lot of time.

On normative issues, I think these are the biggest outstanding ones:

show/hide naming (under discussion in Open UI I guess, not sure how that's going)
Element vs. HTMLElement; i.e., behavior for unknown, SVG, and MathML elements
Some confusion about spec text for some animation pausing stuff
Light dismiss spec intercepting capture events on Document is quite unusual; @annevk's help there would be appreciated steering us in the right direction

source

josepharhar · 2022-10-07T21:05:03Z

show/hide naming (under discussion in Open UI I guess, not sure how that's going)

Looks like we might rename them to popupshow and popuphide: openui/open-ui#607

Element vs. HTMLElement; i.e., behavior for unknown, SVG, and MathML elements

@mfreed7 Why did you implement all of the popup behavior in Element instead of HTMLElement? What do you think the behavior should be for unknown, SVG, and MathML elements?

This was moved from the HTML spec PR for the popup attribute based on this advice: whatwg/html#8221 (comment) TODO add a better description of this

mfreed7 · 2022-10-11T16:07:04Z

@mfreed7 Why did you implement all of the popup behavior in Element instead of HTMLElement? What do you think the behavior should be for unknown, SVG, and MathML elements?

My general logic has been "why not?". I.e. we had the same debate in the TAG about limiting this feature to only some element types. And if there's a good reason to limit it, great. But if the only reason is that we don't know what it'll be used for, I think web developers will figure that out. For example, why not a pop-up SVG? I could think of some use cases where an icon (a "like button" for example) is made with SVG and would like to be a pop-up.

Side note: this is currently broken in Chromium, but pending this discussion, I'll fix that. Just a missing set of rules in the UA stylesheet.

The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33

…y, a=testonly Automatic update from web-platform-tests Do not fire `beforetoggle` asynchronously Per the discussion at [1], we have decided not to fire async beforetoggle events at all. This means that no event will be fired in this case: ```javascript myPopover.showPopover(); myPopover.remove(); ``` [1] whatwg/html#8221 (comment) Bug: 1307772 Change-Id: Ie6d0f24f1181131fd6e15732020c0575cd3ba865 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4146026 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1090506} -- wpt-commits: 7c3aacf158efb050bdb81a1dd97ac324eaee799a wpt-pr: 37814

…e Element reflection, a=testonly Automatic update from web-platform-tests Change popover invoking attributes to use Element reflection Per the conversation at [1], we've decided to make the IDL reflections of the invoking attributes (popovertoggletarget, popovershowtarget, and popoverhidetarget) use Element reflection, and be named accordingly. [1] whatwg/html#8221 (comment) Fixed: 1405856 Bug: 1307772 Change-Id: Iace783795c2db7a19e11fbc4f0c4da33a8765779 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4148056 Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1090582} -- wpt-commits: a0ddc451e02afbbe600c679fe1edab0e4f878ecf wpt-pr: 37815

The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33

…y, a=testonly Automatic update from web-platform-tests Do not fire `beforetoggle` asynchronously Per the discussion at [1], we have decided not to fire async beforetoggle events at all. This means that no event will be fired in this case: ```javascript myPopover.showPopover(); myPopover.remove(); ``` [1] whatwg/html#8221 (comment) Bug: 1307772 Change-Id: Ie6d0f24f1181131fd6e15732020c0575cd3ba865 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4146026 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1090506} -- wpt-commits: 7c3aacf158efb050bdb81a1dd97ac324eaee799a wpt-pr: 37814

…e Element reflection, a=testonly Automatic update from web-platform-tests Change popover invoking attributes to use Element reflection Per the conversation at [1], we've decided to make the IDL reflections of the invoking attributes (popovertoggletarget, popovershowtarget, and popoverhidetarget) use Element reflection, and be named accordingly. [1] whatwg/html#8221 (comment) Fixed: 1405856 Bug: 1307772 Change-Id: Iace783795c2db7a19e11fbc4f0c4da33a8765779 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4148056 Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1090582} -- wpt-commits: a0ddc451e02afbbe600c679fe1edab0e4f878ecf wpt-pr: 37815

The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33

The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4115790 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1096920}

The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4115790 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1096920}

Tests: https://wpt.fyi/results/html/semantics/popovers. Additional context: https://open-ui.org/components/popup.research.explainer. Prior PR: whatwg#8221. Closes whatwg#7785.

Tests: https://wpt.fyi/results/html/semantics/popovers. Additional context: https://open-ui.org/components/popup.research.explainer. Prior PR: #8221. Closes #7785.

…ified, a=testonly Automatic update from web-platform-tests Check popover stack when buttons are modified The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4115790 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1096920} -- wpt-commits: 355b774d54fd4a6fff8165533d731616ac09dc6e wpt-pr: 37594

The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4115790 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1096920}

This is part of the new pop-up attribute: whatwg/html#8221. Co-authored-by: Anne van Kesteren <annevk@annevk.nl> Co-authored-by: Philip Jägenstedt <philip@foolip.org>

…ified, a=testonly Automatic update from web-platform-tests Check popover stack when buttons are modified The popover stack is constructed by attributes set on buttons. When those buttons are modified, it can break connections in the stack. This patch adds checks to spots where buttons can be modified in order to fix up the list by closing all popovers when a connection has been broken. This patch also moves the disabled check for popover*target attributes which Anne asked for here: whatwg/html#8221 (comment) Bug: 1307772, 1408546 Change-Id: I129cf2768abc717292b86ea26f7522019ce36d33 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4115790 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1096920} -- wpt-commits: 355b774d54fd4a6fff8165533d731616ac09dc6e wpt-pr: 37594

This probably should have been added in http://crrev.com/1077035 This was asked for here: whatwg/html#8221 (comment) Bug: 1307772 Change-Id: Iaaf4cb1fa2febb14448845319417f538ac6f9d77 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4086705 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1082836}

…rusted events, a=testonly Automatic update from web-platform-tests Make popup animation listener only use trusted events This was recommended here: whatwg/html#8221 (comment) Bug: 1307772 Change-Id: I602a8ea077fa4253c11eee3ac18c2a302030a379 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3956538 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1061332} -- wpt-commits: 347a664cf4a5156df04a2d309a1400a5c7cc9550 wpt-pr: 36470

… for popup light dismiss, a=testonly Automatic update from web-platform-tests Replace mouse events with pointer events for popup light dismiss This was recommended here: whatwg/html#8221 (comment) This patch also starts firing certain pointer events which otherwise would not get dispatched when there is a popup which is currently being shown. Pointer events don't get dispatched if there is no event listener for them. Bug: 1307772 Change-Id: Ibbe09eb0e639583c94ffbec7b9b335aeba6a70d5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3943910 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1061682} -- wpt-commits: da8b11ac2a5b91930653b186ff30c92613f7c7da wpt-pr: 36469

…povers, a=testonly Automatic update from web-platform-tests Disallow showPopover on transitioning popovers hidePopover doesn't allow transitioning popovers, so we should do the same in showPopover. This inconsistency was pointed out here: whatwg/html#8221 (comment) Bug: 1307772 Change-Id: I3410cd37043100004f02fa3f3be10ec177830324 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4061291 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1077159} -- wpt-commits: 0f44b429e244aa0f99b3b94d3c40c2f6adf9d331 wpt-pr: 37195

…ts into one beforetoggle event, a=testonly Automatic update from web-platform-tests Combine the popoverhide/popove D306 rshow events into one beforetoggle event See [1] and [2] for context. This eliminates the "popoverhide" and "popovershow" events, and replaces them with the "beforetoggle" event, for both of these transitions. The event is a BeforeToggleEvent, which has attributes `currentState` and `newState`, both of which are either "open" or "closed" (and opposite of each other). [1] openui/open-ui#607 (comment) [2] whatwg/html#8221 (comment) Bug: 1307772 Change-Id: I404861a10579365b56e6e8eae7c29f88a5aac166 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4035832 Reviewed-by: David Baron <dbaron@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1080695} -- wpt-commits: 90aec8c944f678c6adc7c48692d7133145018761 wpt-pr: 37031

…pover, a=testonly Automatic update from web-platform-tests Add optional force parameter to togglePopover This was suggested by domenic here: whatwg/html#8221 (comment) Bug: 1307772 Change-Id: Ie371028dd41601f445b7590d0769e6bf101f6072 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4070786 Commit-Queue: Mason Freed <masonf@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1081666} -- wpt-commits: 574f5c564bf1adf6e4f7cfebc266e03d67a4e257 wpt-pr: 37287

…y, a=testonly Automatic update from web-platform-tests Do not fire `beforetoggle` asynchronously Per the discussion at [1], we have decided not to fire async beforetoggle events at all. This means that no event will be fired in this case: ```javascript myPopover.showPopover(); myPopover.remove(); ``` [1] whatwg/html#8221 (comment) Bug: 1307772 Change-Id: Ie6d0f24f1181131fd6e15732020c0575cd3ba865 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4146026 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1090506} -- wpt-commits: 7c3aacf158efb050bdb81a1dd97ac324eaee799a wpt-pr: 37814

…e Element reflection, a=testonly Automatic update from web-platform-tests Change popover invoking attributes to use Element reflection Per the conversation at [1], we've decided to make the IDL reflections of the invoking attributes (popovertoggletarget, popovershowtarget, and popoverhidetarget) use Element reflection, and be named accordingly. [1] whatwg/html#8221 (comment) Fixed: 1405856 Bug: 1307772 Change-Id: Iace783795c2db7a19e11fbc4f0c4da33a8765779 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4148056 Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1090582} -- wpt-commits: a0ddc451e02afbbe600c679fe1edab0e4f878ecf wpt-pr: 37815

josepharhar mentioned this pull request Aug 24, 2022

New feature proposal: Popover API #7785

Closed