Bug 255480

Summary: AX: isSelected AX APIs don't work for some types of display:contents elements
Product: WebKit Reporter: Tyler Wilcock <tyler_w>
Component: AccessibilityAssignee: Tyler Wilcock <tyler_w>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Tyler Wilcock 2023-04-15 01:12:01 PDT
That's because `isSelected` is implemented for AccessibilityRenderObject and not AccessibilityNodeObject despite the fact that the method does not actually require a renderer.
Comment 1 Radar WebKit Bug Importer 2023-04-15 01:12:11 PDT
<rdar://problem/108083208>
Comment 2 Tyler Wilcock 2023-04-15 23:18:47 PDT
Created attachment 465937 [details]
Patch
Comment 3 chris fleizach 2023-04-15 23:54:34 PDT
Comment on attachment 465937 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465937&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:2767
> +    if (!renderer() && !node())

Should we proceed even if we don't have a renderer? for display contents. won't that not have a renderer
Comment 4 Tyler Wilcock 2023-04-16 00:26:37 PDT
(In reply to chris fleizach from comment #3)
> Comment on attachment 465937 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=465937&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:2767
> > +    if (!renderer() && !node())
> 
> Should we proceed even if we don't have a renderer? for display contents.
> won't that not have a renderer
True that display:contents won't have a renderer(), but it will have a node(), so we should be OK here.
Comment 5 chris fleizach 2023-04-16 22:34:28 PDT
Comment on attachment 465937 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465937&action=review

>>> Source/WebCore/accessibility/AccessibilityObject.cpp:2767
>>> +    if (!renderer() && !node())
>> 
>> Should we proceed even if we don't have a renderer? for display contents. won't that not have a renderer
> 
> True that display:contents won't have a renderer(), but it will have a node(), so we should be OK here.

do we need to check renderer() then?
Comment 6 Tyler Wilcock 2023-04-16 23:04:20 PDT
(In reply to chris fleizach from comment #5)
> Comment on attachment 465937 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=465937&action=review
> 
> >>> Source/WebCore/accessibility/AccessibilityObject.cpp:2767
> >>> +    if (!renderer() && !node())
> >> 
> >> Should we proceed even if we don't have a renderer? for display contents. won't that not have a renderer
> > 
> > True that display:contents won't have a renderer(), but it will have a node(), so we should be OK here.
> 
> do we need to check renderer() then?
Maybe not – I mostly tried to preserve the existing logic that would say "is selected" is false if there is no renderer. But I adapted it to display:contents to instead now be "is selected" is false if there is both no renderer and no node. Having one or the other satisfies the check and allows the function to proceed.
Comment 7 EWS 2023-04-17 00:03:45 PDT
Committed 263014@main (0b61863f7ec7): <https://commits.webkit.org/263014@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465937 [details].