Bug 255480 - AX: isSelected AX APIs don't work for some types of display:contents elements
Summary: AX: isSelected AX APIs don't work for some types of display:contents elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-04-15 01:12 PDT by Tyler Wilcock
Modified: 2023-04-17 00:03 PDT (History)
10 users (show)

See Also:


Attachments
Patch (13.76 KB, patch)
2023-04-15 23:18 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].