[Webkit-unassigned] [Bug 236777] AX: List item marker not exposed when not a direct child of a list item

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=236777

--- Comment #7 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Andres Gonzalez from comment #3)
> (In reply to Carlos Garcia Campos from comment #2)
> > Created attachment 452357 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
> +++ a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
> @@ -512,7 +512,15 @@ AccessibilityObject*
> AccessibilityRenderObject::parentObject() const
> 
> +    if (m_renderer->isListMarker()) {
> +        if (auto* listItem =
> ancestorsOfType<RenderListItem>(*m_renderer).first()) {
> +            AccessibilityObject* parent =
> axObjectCache()->getOrCreate(listItem);
> +            if (parent->isListItem())
> +                return parent;
> +        }
> +    }
> +
> 
> Creating new objects here is problematic for isolated tree mode. Not sure it
> makes sense at all to create a new object when we are getting its parent.

We do the same for Menu and MenuBar exceptions, I followed the existing approach. And the non-exceptions path also uses getOrCreate() for the renderParentObject().

> Perhaps 
> 
> auto* parent = axObjectCache()->get(listItem);
> 
> instead?
> 
> Also note that below that block we are getting the cache again:
> 
>      AXObjectCache* cache = axObjectCache();
>      if (!cache)
> 
> Can we avoid the code duplication?

Yes, we should handle the cache being null in all the cases, also for the Menu and MenuBar exceptions.

> -    for (RefPtr<AccessibilityObject> obj = firstChild(); obj; obj =
> obj->nextSibling())
> -        addChild(obj.get());
> +    for (RefPtr<AccessibilityObject> obj = firstChild(); obj; obj =
> obj->nextSibling()) {
> +        if (!obj->renderer()->isListMarker())
> +            addChild(obj.get());
> +    }
> 
> Since you are touching this code, could you please fix the style here? Use
> auto in the for statement, change obj -> object, ...

Sure.

> Can obj->renderer() be null?

No, firstChild() and nextSibling() return nullptr early if renderer is null.

> I wonder if it would be better to special case the list items in this loop
> and add the markers just for them.

I think we always want the marker to be the first child of a list item, that's why I added it after all other children are already added.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220218/27c96f42/attachment.htm>