[Webkit-unassigned] [Bug 179500] AX: AOM: Implement relation type properties

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 328064
  --> https://bugs.webkit.org/attachment.cgi?id=328064
patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:2194
> +        auto elements = AccessibleNode::effectiveElementsValueForElement(*element, propertyKey);
> +        if (elements.size() == 1)
> +            return elements.first();

What happens when elements contain more than one elements?
Should that ever happen? If so, is it really correct to return nullptr in that case?
Where is this behavior spec'ed?

> Source/WebCore/accessibility/AccessibilityObject.cpp:3469
> +    bool idEmpty = id.isEmpty();

Nit: should be idIsEmpty.

> Source/WebCore/accessibility/AccessibleNode.cpp:337
> +AccessibleNode* AccessibleNode::singleRelationValueForProperty(Element& element, AXPropertyName propertyName)

Please return RefPtr<AccessibleNode> for pointer safety.

> Source/WebCore/accessibility/AccessibleNode.cpp:340
> +    if (accessibleNodes.size() == 1)

Is it possible for accessibleNodes to have more than one items? If so, is it really correct to return nullptr.
If that's not possible, we should assert that accessibleNodes.size() is either 0 or 1,
and this check should instead be replaced by if (accessibleNodes.size()).

> Source/WebCore/accessibility/AccessibleNode.cpp:345
> +Vector<AccessibleNode*> AccessibleNode::relationsValueForProperty(Element& element, AXPropertyName propertyName)

Please return Vector<Ref<AccessibleNode>> instead.

> Source/WebCore/accessibility/AccessibleNode.cpp:353
> +Vector<Element*> AccessibleNode::effectiveElementsValueForElement(Element& element, AXPropertyName propertyName)

Please return Vector<Ref<Element>> instead.

> Source/WebCore/accessibility/AccessibleNode.cpp:383
> +AccessibleNode* AccessibleNode::activeDescendant() const

Please return RefPtr<AccessibleNode>

> Source/WebCore/accessibility/AccessibleNode.cpp:482
> +AccessibleNode* AccessibleNode::details() const

Please return RefPtr<AccessibleNode>

> Source/WebCore/accessibility/AccessibleNode.cpp:498
> +AccessibleNode* AccessibleNode::errorMessage() const

Please return RefPtr<AccessibleNode>

-- 
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/20171204/82bba18e/attachment.html>