Bug 255325 - AX: Properly expose lists that have display:contents list items
Summary: AX: Properly expose lists that have display:contents list items
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-12 00:14 PDT by Tyler Wilcock
Modified: 2023-04-12 15:13 PDT (History)
10 users (show)

See Also:


Attachments
Patch (18.59 KB, patch)
2023-04-12 00:21 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (19.10 KB, patch)
2023-04-12 09:59 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-12 00:14:06 PDT
<div role="list" id="list">
    <li id="li1" style="display:contents">One</li>
    <li id="li2"style="display:contents">Two</li>
</div>

We don't expose this as a list which is wrong.
Comment 1 Radar WebKit Bug Importer 2023-04-12 00:14:18 PDT
<rdar://problem/107924637>
Comment 2 Tyler Wilcock 2023-04-12 00:21:07 PDT
Created attachment 465862 [details]
Patch
Comment 3 Andres Gonzalez 2023-04-12 07:26:28 PDT
(In reply to Tyler Wilcock from comment #2)
> Created attachment 465862 [details]
> Patch

--- a/Source/WebCore/accessibility/AXObjectCache.cpp
+++ b/Source/WebCore/accessibility/AXObjectCache.cpp

-    RefPtr<AccessibilityObject> newObj = createFromNode(node);
+    RefPtr<AccessibilityObject> newObj = createFromNode(*node);

RefPtr<AccessibilityObject> newObj --> RefPtr newObject

+    if (auto* node = this->node())
+        return node->hasTagName(ulTag);
+    return false;

I prefer the two liner idiom:

    auto* node = this->node();
    return node ? node->hasTagName(ulTag) : false;

+    if (auto* node = this->node())
+        return node->hasTagName(olTag);
+    return false;

Same as above.

+    if (auto* node = this->node())
+        return node->hasTagName(dlTag);
+    return false;

Same as above.

         else if (child->roleValue() == AccessibilityRole::ListItem) {
-            RenderObject* listItem = child->renderer();
-            if (!listItem)
-                continue;
-
             // Rendered list items always count.


Does this comment make sense any more?
Comment 4 Tyler Wilcock 2023-04-12 09:59:59 PDT
Created attachment 465868 [details]
Patch
Comment 5 Tyler Wilcock 2023-04-12 10:01:29 PDT
>          else if (child->roleValue() == AccessibilityRole::ListItem) {
> -            RenderObject* listItem = child->renderer();
> -            if (!listItem)
> -                continue;
> -
>              // Rendered list items always count.
> Does this comment make sense any more?
Just re-read the function and I think it still does since the purpose of the if-statement is to find rendered list items. Fixed all other comments, thanks!
Comment 6 EWS 2023-04-12 15:13:34 PDT
Committed 262889@main (f49113b35bbd): <https://commits.webkit.org/262889@main>

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