Summary: | title attribute is not exposed as the AXDescription on AXImage when there is no other fallback content | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||||||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dglazkov, dmazzoni, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
chris fleizach
2012-09-11 11:29:07 PDT
Created attachment 163417 [details]
patch
Comment on attachment 163417 [details] patch Attachment 163417 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13826335 Comment on attachment 163417 [details] patch Attachment 163417 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13819431 Comment on attachment 163417 [details] patch Attachment 163417 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13832041 Created attachment 163418 [details]
patch
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 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. 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 (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 Created attachment 163447 [details]
patch for landing
Created attachment 163448 [details]
patch for landing
|