Revamp the popover target attributes by annevk · Pull Request #8962 · whatwg/html

annevk · 2023-03-01T01:08:37Z

Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target.

Tests: web-platform-tests/wpt#38701.

Fixes #8894.

At least two implementers are interested (and none opposed):
- WebKit
- Chromium
Tests are written and can be reviewed and commented upon at:
- Change popover invoking attributes web-platform-tests/wpt#38701
Implementation bugs are filed:
- Chromium: N/A
- Gecko: N/A
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=252078
MDN issue is filed: Document HTML popover attribute (and related features) mdn/content#25125

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

/form-elements.html ( diff )
/indices.html ( diff )
/input.html ( diff )
/popover.html ( diff )

josepharhar

Thank you for speccing this!

annevk · 2023-03-02T12:40:03Z

Thanks, @domenic care to rubberstamp/review?

mfreed7

Looks good to me. One correction.

domenic

"limited to only known values" is the biggest question here. I thought that was best practice these days.

domenic · 2023-03-03T06:57:13Z

+    data-x="attr-popovertarget">popovertarget</code> attribute pointing to a popover. In this case,
+    the popover is the "child", and the DOM-contained popover of the invoking element is the
+    "parent". The invoker must be in a popover and reference an open popover.</p></li>
   </ol>


Not necessarily related to this PR, but I find this note confusing in how it conflates the concepts of "ancestor" and "parent". Might be good to drive-by fix.

I will push some nits, but I'm not sure how to fix this properly. Maybe you or @josepharhar could push a fixup?

I'm not sure if the "parent" is really supposed to be the topmost ancestor in this note, I think it's more generally describing how the tree of popovers is created by describing the relationships between two adjacent popovers in the popover tree.
@mfreed7 might have additional thoughts

Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target. Tests: web-platform-tests/wpt#38701. Fixes #8894.

Fixes #8983.

annevk · 2023-03-07T20:33:49Z

@mfreed7 @josepharhar @domenic ping. It would be good to get this in given that the specification is out-of-date with respect to implementations.

https://bugs.webkit.org/show_bug.cgi?id=252078 rdar://105612966 Reviewed by Chris Dumez. This adds support for the popoverTargetElement & popoverTargetAction IDL attributes, which map to popovertarget & popovertargetaction HTML attributes. Spec change is at whatwg/html#8962 , WPT have already been updated by a Chromium commit to match. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-invoking-attribute-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-stacking-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-target-element-disabled-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-top-layer-combinations-expected.txt: * LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt: * LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-2-expected.txt: * LayoutTests/platform/glib/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt: * LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/popovers/popover-light-dismiss-expected.txt: * LayoutTests/platform/ios/TestExpectations: Rebaseline tests with progressions. Remaining failures are either due to these unimplemented parts: - invoker relationship removal handling (not standardized?) - popover focusing steps (https://bugs.webkit.org/show_bug.cgi?id=253231) - anchor attribute (not standardized) * Source/WebCore/CMakeLists.txt: * Source/WebCore/DerivedSources-input.xcfilelist: * Source/WebCore/DerivedSources-output.xcfilelist: * Source/WebCore/DerivedSources.make: Update list of built files. * Source/WebCore/dom/Document.cpp: (WebCore::Document::handlePopoverLightDismiss): Check for invokers as described in algorithm in: https://html.spec.whatwg.org/#topmost-clicked-popover * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::addRelations): * Source/WebCore/accessibility/AccessibilityObject.cpp: * Source/WebCore/dom/Element.cpp: (WebCore::Element::isElementReflectionAttribute): (WebCore::Element::isElementsArrayReflectionAttribute): (WebCore::Element::attributeChanged): (WebCore::Element::getElementAttribute const): (WebCore::Element::setElementAttribute): (WebCore::Element::getElementsArrayAttribute const): (WebCore::Element::setElementsArrayAttribute): * Source/WebCore/dom/Element.h: Add IDL element reflection support for popovertarget attribute. Simplified settings checks. * Source/WebCore/dom/PopoverData.h: (WebCore::PopoverData::invoker const): (WebCore::PopoverData::setInvoker): Store invoker in popover data. * Source/WebCore/html/HTMLAttributeNames.in: Define popovertargetAttr & popovertargetActionAttr. * Source/WebCore/html/HTMLButtonElement.cpp: (WebCore::HTMLButtonElement::defaultEventHandler): Invoke popover activation behavior. * Source/WebCore/html/HTMLButtonElement.idl: Use PopoverInvokerElement mixin. * Source/WebCore/html/HTMLElement.cpp: (WebCore::topmostPopoverAncestor): Check for invokers as defined in the spec: https://html.spec.whatwg.org/#topmost-popover-ancestor (WebCore::HTMLElement::hidePopoverInternal): Clear invoker as defined in the spec. * Source/WebCore/html/HTMLFormControlElement.cpp: (WebCore::toggleAtom): (WebCore::showAtom): (WebCore::hideAtom): (WebCore::HTMLFormControlElement::popoverTargetElement const): Implement https://html.spec.whatwg.org/#popover-target-element (WebCore::HTMLFormControlElement::popoverTargetAction const): Implement attribute reflection. (WebCore::HTMLFormControlElement::handlePopoverTargetAction const): Implement activation behavior: https://html.spec.whatwg.org/#popover-target-attribute-activation-behavior * Source/WebCore/html/HTMLFormControlElement.h: (WebCore::HTMLFormControlElement::setPopoverTargetAction): Implement attribute reflection. * Source/WebCore/html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::defaultEventHandler): Invoke popover activation behavior. * Source/WebCore/html/HTMLInputElement.idl: Use PopoverInvokerElement mixin. * Source/WebCore/html/PopoverInvokerElement.idl: Added. Canonical link: https://commits.webkit.org/261346@main

domenic

LGTM with nits

annevk added normative change topic: popover The popover attribute and friends labels Mar 1, 2023

annevk force-pushed the annevk/popovertarget branch from 9e56fd0 to e4bb54f Compare March 1, 2023 01:12

annevk assigned nt1m, josepharhar and mfreed7 Mar 1, 2023

josepharhar reviewed Mar 1, 2023

View reviewed changes

Comment thread source Outdated

josepharhar reviewed Mar 1, 2023

View reviewed changes

Comment thread source Outdated

annevk requested review from josepharhar, mfreed7 and nt1m March 1, 2023 17:23

annevk unassigned nt1m, josepharhar and mfreed7 Mar 1, 2023

annevk mentioned this pull request Mar 1, 2023

Adds proposed popover and related attributes w3c/html-aam#446

Closed

josepharhar approved these changes Mar 2, 2023

View reviewed changes

annevk requested a review from domenic March 2, 2023 12:39

annevk mentioned this pull request Mar 2, 2023

Correct WAI-ARIA integration for custom elements #8974

Merged

This comment was marked as resolved.

Sign in to view

mfreed7 approved these changes Mar 3, 2023

View reviewed changes

Comment thread source Outdated

domenic reviewed Mar 3, 2023

View reviewed changes

annevk force-pushed the annevk/popovertarget branch from 4075b53 to 2f0b55e Compare March 3, 2023 19:53

annevk requested a review from domenic March 3, 2023 19:56

annevk mentioned this pull request Mar 4, 2023

Issues with "Check popover validity" in popover target attribute activation behavior #8983

Closed

nt1m mentioned this pull request Mar 7, 2023

[popover] Implement popovertarget & popovertargetaction attributes WebKit/WebKit#11138

Merged

annevk added 2 commits March 6, 2023 17:31

review + nits

96b06f1

annevk added 3 commits March 6, 2023 17:31

Update source

f285948

Update source

18e3285

Correct check popover validity checks in activation behavior

c75fffc

Fixes #8983.

align with element-reflecting change

e66faab

annevk force-pushed the annevk/popovertarget branch from 610363f to e66faab Compare March 7, 2023 01:37

annevk mentioned this pull request Mar 7, 2023

Document HTML popover attribute (and related features) mdn/content#25125

Closed

annevk requested review from josepharhar and mfreed7 March 7, 2023 01:46

nt1m mentioned this pull request Mar 7, 2023

[popover] Implement popover focusing steps WebKit/WebKit#11159

Merged

domenic approved these changes Mar 8, 2023

View reviewed changes

Comment thread source Outdated

Comment thread source Outdated

review, align popover attribute at the same time

bd5cec6

annevk merged commit dc27ce8 into main Mar 8, 2023

annevk deleted the annevk/popovertarget branch March 8, 2023 14:42

YusukeHirao mentioned this pull request Mar 9, 2023

Update the popover target attributes markuplint/markuplint#746

Closed

YusukeHirao mentioned this pull request Sep 18, 2023

Update Popover API markuplint/markuplint#1152

Closed

Navigation Menu

Repository navigation

Revamp the popover target attributes - #8962

Revamp the popover target attributes#8962
annevk merged 7 commits into
mainfrom
annevk/popovertarget

annevk commented Mar 1, 2023 •

edited by pr-preview Bot

Loading

Uh oh!

Uh oh!

Uh oh!

josepharhar left a comment

Uh oh!

annevk commented Mar 2, 2023

Uh oh!

This comment was marked as resolved.

mfreed7 left a comment

Uh oh!

Uh oh!

domenic left a comment

Uh oh!

domenic Mar 3, 2023

Uh oh!

annevk Mar 3, 2023

Uh oh!

josepharhar Mar 7, 2023

Uh oh!

Uh oh!

Uh oh!

annevk commented Mar 7, 2023

Uh oh!

domenic left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

Conversation

annevk commented Mar 1, 2023 • edited by pr-preview Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josepharhar left a comment

Choose a reason for hiding this comment

Uh oh!

annevk commented Mar 2, 2023

Uh oh!

This comment was marked as resolved.

mfreed7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

domenic Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

annevk Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

josepharhar Mar 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

annevk commented Mar 7, 2023

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

annevk commented Mar 1, 2023 •

edited by pr-preview Bot

Loading