[webkit-reviews] review granted: [Bug 82819] search field on Google.com does not have the AXSearchField subrole : [Attachment 134948] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 82819: search field on Google.com does not have the AXSearchField subrole
https://bugs.webkit.org/show_bug.cgi?id=82819

Attachment 134948: patch
https://bugs.webkit.org/attachment.cgi?id=134948&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=134948&action=review


> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:506
> +    // Some websites don't label their search fields as search, so we should
fallback to some heuristics.

This comment doesn’t explain why these heuristics are helpful. Nor why these
heuristics are sufficient.

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:508
> +    String nameAttribute = getAttribute(nameAttr);

const AtomicString& nameAttribute = fastGetAttribute(nameAttr);

> LayoutTests/platform/mac/accessibility/search-subrole-expected.txt:15
> +PASS nosearch.subrole is 'AXSubrole: '

Could use more coverage for the negative. For example, an element that is not
in any form.

Need coverage for the “not case sensitive” aspect. All these are all lowercase
so the “false” for case sensitive is not tested at all.