[Webkit-unassigned] [Bug 229556] AX: Make PDFs loaded via <embed> accessible

[bugzilla-daemon at webkit.org]

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

--- Comment #8 from chris fleizach <cfleizach at apple.com> ---
Comment on attachment 437632
  --> https://bugs.webkit.org/attachment.cgi?id=437632
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.h:450
> +    PluginViewBase* pluginViewBase() const override { return nullptr; }

while the type is a pluginViewBase, the name seems like it could be just "pluginView"

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1356
> +    if (pluginViewBase())

if (auto pluginView = pluginViewBase())
   return !pluginView->accessibilityObject()

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2010
> +    auto* thisWidget = widget();

if (auto *widget = this->widget())
   return is<PluginViewBase>(widget) ? downcast<PluginViewBase>(widget) : nullptr;

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1662
> +        return @[pluginViewBase->accessibilityObject()];

we should verify accessibilityObject() is not null before putting it in an array

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2951
> +        if (axObject->pluginViewBase())

if (auto *pluginView = axObject->pluginViewBase())
   return pluginView->....

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2952
> +            return axObject->pluginViewBase()->accessibilityHitTest(IntPoint(point));

do we want to handle the case where the returned object is accessibilityIgnored? should we do the default hit test in that case?

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3705
> +    return axObject->pluginViewBase()

you can do a local cache of pluginViewBase since you check it twice

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:255
> +    if ([attribute isEqualToString:axTestObjectTypeAttributeName])

wonder if we should just add a subrole type for this like "AXPDFPluginSubrole" instead of exposing a testing attribute for all clients

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:3069
> +    if (m_accessibilityObject)

this check not necessary, since if it's nil calling axHitTest on it will return nil anyway

> Source/WebKit/WebProcess/Plugins/Plugin.h:41
> +OBJC_CLASS NSArray;

doesn't look like we need to add NSArray here

> LayoutTests/accessibility/mac/basic-embed-pdf-accessibility.html:28
> +            pdfEmbedElement = body.uiElementForSearchPredicate(embedContainer, true, "AXGroupSearchKey", "", false);

can you also do a hit test into the embedded object to ensure we return an element

> LayoutTests/accessibility/mac/basic-embed-pdf-accessibility.html:33
> +            pdfAxObject = pdfEmbedElement.childAtIndex(0);

can you verify reaching into the PDF document to get a node

> LayoutTests/accessibility/mac/basic-embed-pdf-accessibility.html:36
> +

can you verify the parentage is correct for this object

-- 
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/20210908/ee4c85ab/attachment.htm>