Bug 129939 – AX: add AccessibilityObject::computedLabelString() for WebAXI

• Component: Accessibility
• Status: RESOLVED FIXED

[James Craig]

AX: add AccessibilityObject::computedLabelString() for WebAXI

Wrap up the existing label methods into a single computedLabelString() in AccessibilityObject. Among other things, this will be used for the Web Accessibility Node Inspector.

[James Craig]

<rdar://problem/16267672>

[Samuel White]

Created attachment 233378 [details]
Conversation starter patch.

Just a simple initial patch to hook things up and give us a jumping off point.

[WebKit Commit Bot]

Attachment 233378 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1341:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1342:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1433:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1444:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Samuel White]

Created attachment 233380 [details]
Fixing style issues.

Fixing the relevant style issues.

[WebKit Commit Bot]

Attachment 233380 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1341:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1342:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Samuel White]

(In reply to comment #5)
> Attachment 233380 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1341:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1342:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Total errors found: 2 in 6 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

These errors can be ignored. Same indentation and number of leading spaces as surrounding lines.

[Samuel White]

(In reply to comment #6)
> (In reply to comment #5)
> > Attachment 233380 [details] [details] did not pass style-queue:
> > 
> > 
> > ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1341:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> > ERROR: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1342:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> > Total errors found: 2 in 6 files
> > 
> > 
> > If any of these errors are false positives, please file a bug against check-webkit-style.
> 
> These errors can be ignored. Same indentation and number of leading spaces as surrounding lines.

Style failure bug: https://bugs.webkit.org/show_bug.cgi?id=134074

[Build Bot]

Comment on attachment 233380 [details]
Fixing style issues.

Attachment 233380 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5546361298616320

New failing tests:
accessibility/plugin.html
platform/mac/accessibility/aria-columnrowheaders.html
platform/mac/accessibility/document-attributes.html
accessibility/image-link.html
platform/mac/accessibility/document-links.html
accessibility/table-attributes.html
accessibility/table-sections.html
platform/mac/accessibility/internal-link-anchors.html
accessibility/table-one-cell.html
accessibility/table-detection.html
platform/mac/accessibility/bounds-for-range.html
accessibility/table-cells.html
accessibility/image-map2.html
accessibility/table-cell-spans.html
accessibility/table-with-rules.html
accessibility/transformed-element.html
accessibility/internal-link-anchors2.html
accessibility/lists.html

[Build Bot]

Created attachment 233397 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[chris fleizach]

Comment on attachment 233380 [details]
Fixing style issues.

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

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1433
> +    if (accessibilityText.size() > 0 && accessibilityText[0].text.length() > 0)

don't compare against 0 per webkit style

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1441
> +    Vector<AccessibilityText> accessibilityText;

is there a reason the computed name is the same as the computed description?

> Source/WebCore/accessibility/AccessibilityObject.h:652
> +    virtual String accessibilityComputedDescription() const { return String(); }

these methods might be able to be put into AccessibilityObject.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1342
> +                      NSAccessibilityComputedName,

since these are private to WebKit, best not to put them in the list of supported attributes.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2272
> +        return (NSString *)m_object->accessibilityComputedDescription();

I don't think you need to cast to NSString, that should just work

[Samuel White]

(In reply to comment #10)
> (From update of attachment 233380 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=233380&action=review
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1433
> > +    if (accessibilityText.size() > 0 && accessibilityText[0].text.length() > 0)
> 
> don't compare against 0 per webkit style

Thanks.

> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1441
> > +    Vector<AccessibilityText> accessibilityText;
> 
> is there a reason the computed name is the same as the computed description?

The alternativeText method is our best existing approximation of the alt text comp algorithm. There is work to be done to meet the slightly different needs of computed description and computed name. This initial patch is really just a jumping off point to discuss which bits we need to expand on to enable some additional inspector enhancements that are in flight.

> 
> > Source/WebCore/accessibility/AccessibilityObject.h:652
> > +    virtual String accessibilityComputedDescription() const { return String(); }
> 
> these methods might be able to be put into AccessibilityObject.

Since the accessibility text vector we build requires a node obj I believe these will to. If that doesn't turn out to be the case, I will move them.

> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1342
> > +                      NSAccessibilityComputedName,
> 
> since these are private to WebKit, best not to put them in the list of supported attributes.

Makes sense.

> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2272
> > +        return (NSString *)m_object->accessibilityComputedDescription();
> 
> I don't think you need to cast to NSString, that should just work

Just a personal pref. to be explicit. Will remove.

Thanks for the initial feedback Chris!

James, given the huge scope, I think we need to focus on expanding the test(s) BEFORE we iterate on the machinery as needed. Any links to existing w3c tests for the alt text comp spec. would be a huge advantage. Thanks.

[James Craig]

Would you prefer a link to the raw Mercurial repo (there are error test cases in it) or would you prefer I put together a set of markup examples in this format? You can always comment the ones that don’t work yet.

[James Craig]

“Error test cases” means some of the cases are invalid. I’ve corrected most when I find them, but there are definitely some sleepers left in the 700+ test cases. Unfortunately not all te contributors adhered to the desired level of quality.

[James Craig]

Here’s the PFWG test case repo:
hg clone https://dvcs.w3.org/hg/pfwg/

[Build Bot]

Comment on attachment 233380 [details]
Fixing style issues.

Attachment 233380 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6220546610036736

New failing tests:
accessibility/plugin.html
platform/mac/accessibility/aria-columnrowheaders.html
platform/mac/accessibility/document-attributes.html
accessibility/image-link.html
platform/mac/accessibility/document-links.html
accessibility/table-attributes.html
accessibility/table-sections.html
platform/mac/accessibility/internal-link-anchors.html
accessibility/table-one-cell.html
accessibility/table-detection.html
platform/mac/accessibility/bounds-for-range.html
accessibility/table-cells.html
accessibility/image-map2.html
accessibility/table-cell-spans.html
accessibility/table-with-rules.html
accessibility/transformed-element.html
accessibility/internal-link-anchors2.html
accessibility/lists.html

[Build Bot]

Created attachment 233400 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 233380 [details]
Fixing style issues.

Attachment 233380 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6424594600689664

New failing tests:
accessibility/plugin.html
platform/mac/accessibility/aria-columnrowheaders.html
platform/mac/accessibility/document-attributes.html
accessibility/image-link.html
platform/mac/accessibility/document-links.html
accessibility/table-attributes.html
accessibility/transformed-element.html
accessibility/table-sections.html
platform/mac/accessibility/internal-link-anchors.html
accessibility/table-one-cell.html
accessibility/table-detection.html
platform/mac/accessibility/bounds-for-range.html
accessibility/table-cells.html
accessibility/image-map2.html
accessibility/table-cell-spans.html
accessibility/table-with-rules.html
media/W3C/video/src/src_reflects_attribute_not_source_elements.html
accessibility/internal-link-anchors2.html
accessibility/lists.html

[Build Bot]

Created attachment 233428 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[James Craig]

Chris, can you take and finish this one? It's blocking one of my AXI bugs. Thx.

[chris fleizach]

Created attachment 235995 [details]
simplified patch

[James Craig]

Thanks

[WebKit Commit Bot]

Comment on attachment 235995 [details]
simplified patch

Clearing flags on attachment: 235995

Committed r172021: <http://trac.webkit.org/changeset/172021>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.