[webkit-reviews] review denied: [Bug 101650] AX: textUnderElement should consider alt text, but skip links and controls : [Attachment 178868] Patch

[bugzilla-daemon at webkit.org]

Dominic Mazzoni <dmazzoni at google.com> has denied Dominic Mazzoni
<dmazzoni at google.com>'s request for review:
Bug 101650: AX: textUnderElement should consider alt text, but skip links and
controls
https://bugs.webkit.org/show_bug.cgi?id=101650

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

------- Additional Comments from Dominic Mazzoni <dmazzoni at google.com>
OK, I renamed the bug to capture the essence of the issue and I've got all
tests passing, will upload momentarily.

Please take a close look at the test expectations I needed to change. I think
most are an improvement, but a couple are unclear. Let me know if you think we
should improve the accessible text for a couple of those corner cases or if
they look fine.

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


>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1429
>> +static bool shouldRecurseIntoObjectForInnerText(AccessibilityObject* obj)
> 
> what do you think of the name shouldUseAccessiblityObjectInnerText()

Sure. I added a lot more comments too.

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1431
>> +	if (obj->canSetFocusAttribute())
> 
> some comments about why we shouldn't recurse for canSetFocus() and the other
if() check would be good here

Done.