title attribute is not exposed as the AXDescription on AXImage when there is no other fallback content

[chris fleizach]

The bug here is that the title attribute does not become the label on AXImage if there is no alt tag.

[chris fleizach]

Created attachment 163417 [details]
patch

[Early Warning System Bot]

Comment on attachment 163417 [details]
patch

Attachment 163417 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13826335

[Early Warning System Bot]

Comment on attachment 163417 [details]
patch

Attachment 163417 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13819431

[WebKit Review Bot]

Comment on attachment 163417 [details]
patch

Attachment 163417 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13832041

[chris fleizach]

Created attachment 163418 [details]
patch

[Jon Honeycutt]

Comment on attachment 163418 [details]
patch

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

> LayoutTests/accessibility/img-fallsback-to-title.html:12
> +    <img title="test1" height="100" width="100">
> +    <img alt="alt" title="test2" height="100" width="100">
> +    <div role="img" title="test3" width="100" height="100">test</div>
> +    <div role="img" alt="alt" title="test4" width="100" height="100">test</div>

Please add a test for the empty alt attribute case if we don't already have one.

r=me

[Darin Adler]

Comment on attachment 163418 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1508
> -    Node* node = m_renderer->node();
>      if (isImage() || isInputImage() || isNativeImage() || isCanvas()) {
> -        if (node && node->isHTMLElement()) {
> -            const AtomicString& alt = toHTMLElement(node)->getAttribute(altAttr);
> -            if (alt.isEmpty())
> -                return String();
> +
> +        // Images should use alt as long as the attribute is present, even if empty.
> +        // Otherwise, it should fallback to other methods, like the title attribute.
> +        const AtomicString& alt = getAttribute(altAttr);
> +        if (!alt.isNull())
>              return alt;
> -        }
>      }

I understand the change from isEmpty to isNull. I don’t understand the removal of the HTMLElement check, nor does the test case seem to cover this.

[Dominic Mazzoni]

Comment on attachment 163418 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1508
>>      }
> 
> I understand the change from isEmpty to isNull. I don’t understand the removal of the HTMLElement check, nor does the test case seem to cover this.

AccessibilityObject::getAttribute already does the HTMLElement check

[chris fleizach]

(In reply to comment #6)
> (From update of attachment 163418 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163418&action=review
> 
> > LayoutTests/accessibility/img-fallsback-to-title.html:12
> > +    <img title="test1" height="100" width="100">
> > +    <img alt="alt" title="test2" height="100" width="100">
> > +    <div role="img" title="test3" width="100" height="100">test</div>
> > +    <div role="img" alt="alt" title="test4" width="100" height="100">test</div>
> 
> Please add a test for the empty alt attribute case if we don't already have one.
> 
> r=me

Will add. Thanks

[chris fleizach]

Created attachment 163447 [details]
patch for landing

[chris fleizach]

Created attachment 163448 [details]
patch for landing

[chris fleizach]

http://trac.webkit.org/changeset/128227

[chris fleizach]

rdar://12061421