[Webkit-unassigned] [Bug 147295] AX: VoiceOver unable to access content in malformed trees

[bugzilla-daemon at webkit.org]

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

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

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

in general, i think we don't want to add new attributes. we want to keep our logic inside ::roleValue() and determineRoleValue()

> Source/WebCore/accessibility/AccessibilityRenderObject.h:214
> +    virtual bool shouldIgnoreAttributeRole() const { return false; }

I doubt we need either of these methods.

> Source/WebCore/accessibility/AccessibilityTree.cpp:62
> +    m_ariaRole = determineAriaRoleAttribute();

it doesn't look like you use this anywhere.

for 1, if the role is not a "tree" role we shouldn't proceed any further

> Source/WebCore/accessibility/AccessibilityTree.cpp:65
> +    m_role = TreeRole;

seems like we should just override canHaveChildren() and return true

> Source/WebCore/accessibility/AccessibilityTree.cpp:74
> +        if (child->isGroup()) {

can we have groups of groups of groups of tree items?

> Source/WebCore/accessibility/AccessibilityTree.cpp:77
> +                    m_isMalformed = true;

this should just set some local variable. we don't need an instance variable to store this

> Source/WebCore/accessibility/AccessibilityTree.cpp:84
> +    // After validating the tree, we want the children to reflect the correct role

this comment is not necessary. it is self evident from the code

> Source/WebCore/accessibility/AccessibilityTreeItem.h:42
> +        virtual bool shouldIgnoreAttributeRole() const override { return !m_isTreeItemValid; }

i don't think this concept is a good one. i think the role should just reflect what the object is. we don't want to have to check another attribute to figure out what the role is

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2059
> +            return @"AXMalformed";

this is not something we want to return. if the object is not a tree, it should just revert to what it normally would be, which will be taken care of by code below

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150727/06425ab9/attachment.html>