add/revise popover related attribute mappings #481
Conversation
this commit supersedes #446 TODO: finalize `popover` attribute mappings
…state to assistive technologies https://bugs.webkit.org/show_bug.cgi?id=257666 rdar://105425310 Reviewed by Chris Fleizach. Per w3c/html-aam#481, buttons with the `popovertarget` attribute and valid associated popover should expose expanded state to assistive technologies. This commit implements that, and also submits a notification to ATs when a popover is expanded and collapsed. * LayoutTests/accessibility/mac/expanded-notification-expected.txt: * LayoutTests/accessibility/mac/expanded-notification.html: Popover testcase added. * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::onPopoverTargetToggle): * Source/WebCore/accessibility/AXObjectCache.h: * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::popoverTargetElement const): * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::supportsExpanded const): (WebCore::AccessibilityObject::isExpanded const): * Source/WebCore/accessibility/AccessibilityObject.h: (WebCore::AccessibilityObject::popoverTargetElement const): * Source/WebCore/html/HTMLFormControlElement.cpp: (WebCore::HTMLFormControlElement::handlePopoverTargetAction const): Canonical link: https://commits.webkit.org/264852@main
…state to assistive technologies https://bugs.webkit.org/show_bug.cgi?id=257666 rdar://105425310 Reviewed by Chris Fleizach. Per w3c/html-aam#481, buttons with the `popovertarget` attribute and valid associated popover should expose expanded state to assistive technologies. This commit implements that, and also submits a notification to ATs when a popover is expanded and collapsed. * LayoutTests/accessibility/mac/expanded-notification-expected.txt: * LayoutTests/accessibility/mac/expanded-notification.html: Popover testcase added. * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::onPopoverTargetToggle): * Source/WebCore/accessibility/AXObjectCache.h: * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::popoverTargetElement const): * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::supportsExpanded const): (WebCore::AccessibilityObject::isExpanded const): * Source/WebCore/accessibility/AccessibilityObject.h: (WebCore::AccessibilityObject::popoverTargetElement const): * Source/WebCore/html/HTMLFormControlElement.cpp: (WebCore::HTMLFormControlElement::handlePopoverTargetAction const): Canonical link: https://commits.webkit.org/264852@main
|
made related follow up #505 |
index.html
Outdated
| <tr> | ||
| <th><a href="https://msdn.microsoft.com/en-us/library/dd373608%28v=VS.85%29.aspx">MSAA</a> + <a href="http://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/">IAccessible2</a></th> | ||
| <td> | ||
| <span class="type">Relations:</span> `IA2_RELATION_DETAILS_FOR` points to invoking element |
There was a problem hiding this comment.
It feels weird that we express this as a platform specific relation, but we express popovertarget's details relation as a cross-platform aria-details relation. Is this just because we have no way of expressing reverse relations for ARIA mappings and we want to be explicit here? I wonder whether it'd be better to link to a more general note about implicit reverse relations or something like that.
There was a problem hiding this comment.
I'd be fine with either replacing the cross-platform aria-details relation with the specific platform mappings, or keeping the cross-platform relation and removing the platform specific and replacing with a note about the reverse relations.
over in #359 (which incase you missed, i pinged you for a review) we use the platform specific reverse relations for figure/figcaption. So we can do that here too, or similarly change over in that PR.
Whatever makes the most sense for those that need to read the spec is what I want to do, so just let me know which is preferred.
index.html
Outdated
| <tr> | ||
| <th><a href="https://msdn.microsoft.com/en-us/library/dd373608%28v=VS.85%29.aspx">MSAA</a> + <a href="http://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/">IAccessible2</a></th> | ||
| <td> | ||
| <span class="type">Relations:</span> `IA2_RELATION_DETAILS_FOR` points to invoking element |
There was a problem hiding this comment.
I assume this shouldn't happen if we don't expose aria-details due to the invoking element being an immediate sibling or having popovertargetaction=hide? Again, hopefully common sense will prevail, but probably best not to rely on that.
| </p> | ||
| <p> | ||
| If the associated element is an accessibility ancestor of the element with the `popovertarget` attribute: | ||
| <a class="core-mapping" href="#ariaExpandedUndefined">`aria-expanded=undefined`</a> |
There was a problem hiding this comment.
This doesn't appear to be implemented in Chromium:
data:text/html,<button popovertarget="pop">tog</button><div id="pop" popover>pop<button popovertarget="pop" popovertargetaction="hide">hide
When the popover is shown, the "hide" button gets the expanded state. If I'm reading this correctly, it shouldn't.
Just to double check, is this definitely what we want and it just hasn't been done in Chromium yet? Or is this still an open question we need to sort out in this PR?
There was a problem hiding this comment.
yes it should not be getting the expanded state for the hide button, this will need to be adjusted in chromium
|
closing - will be merged via w3c/aria#2219 |
this commit supersedes #446
TODO: finalize
popoverattribute mappingsPreview | Diff