Bug 155469 - AX: Add aria-current to global ARIA attributes list
Summary: AX: Add aria-current to global ARIA attributes list
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-03-14 15:08 PDT by Nan Wang
Modified: 2016-03-17 10:32 PDT (History)
13 users (show)

See Also:


Attachments
patch (4.91 KB, patch)
2016-03-14 15:36 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
patch (3.72 KB, patch)
2016-03-15 19:26 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (901.48 KB, application/zip)
2016-03-15 20:32 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nan Wang 2016-03-14 15:08:22 PDT
Sometimes aria-current information is not spoken by VoiceOver because the aria-current is set to the parent element.
For instance, aria-current is set to a span tag but VoiceOver is reading its child text element, which has no aria-current information associated.
Comment 1 Radar WebKit Bug Importer 2016-03-14 15:09:12 PDT
<rdar://problem/25153528>
Comment 2 Nan Wang 2016-03-14 15:36:54 PDT
Created attachment 274039 [details]
patch

initial patch
Comment 3 chris fleizach 2016-03-14 17:37:25 PDT
Comment on attachment 274039 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:1881
> +    // If empty or missing, we should walk the parent chain to find the current status.

should we do this for every object, or should VoiceOver be responsible for checking the parent chain for the presence of aria-current?
i feel like the later might be more performant and more accurate.
Comment 4 Nan Wang 2016-03-14 17:52:05 PDT
Comment on attachment 274039 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityObject.cpp:1881
>> +    // If empty or missing, we should walk the parent chain to find the current status.
> 
> should we do this for every object, or should VoiceOver be responsible for checking the parent chain for the presence of aria-current?
> i feel like the later might be more performant and more accurate.

I did have the same concern when implementing this. I'll try the VO approach too, but even in that case, we still need to check every object, right?
Comment 5 chris fleizach 2016-03-14 17:54:44 PDT
Comment on attachment 274039 [details]
patch

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

>>> Source/WebCore/accessibility/AccessibilityObject.cpp:1881
>>> +    // If empty or missing, we should walk the parent chain to find the current status.
>> 
>> should we do this for every object, or should VoiceOver be responsible for checking the parent chain for the presence of aria-current?
>> i feel like the later might be more performant and more accurate.
> 
> I did have the same concern when implementing this. I'll try the VO approach too, but even in that case, we still need to check every object, right?

true but VO only has to do it when it needs to output the info. Does webkit end up calling this method for every object when its retrieved? or does it also only return this info when explicitly asked.

I feel its also slightly more accurate to only expose this on the actual element that does expose it.
Comment 6 Nan Wang 2016-03-14 18:10:46 PDT
Comment on attachment 274039 [details]
patch

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

>>>> Source/WebCore/accessibility/AccessibilityObject.cpp:1881
>>>> +    // If empty or missing, we should walk the parent chain to find the current status.
>>> 
>>> should we do this for every object, or should VoiceOver be responsible for checking the parent chain for the presence of aria-current?
>>> i feel like the later might be more performant and more accurate.
>> 
>> I did have the same concern when implementing this. I'll try the VO approach too, but even in that case, we still need to check every object, right?
> 
> true but VO only has to do it when it needs to output the info. Does webkit end up calling this method for every object when its retrieved? or does it also only return this info when explicitly asked.
> 
> I feel its also slightly more accurate to only expose this on the actual element that does expose it.

Webkit is calling this method for every object, but only expose the attribute when result is not false. I think I should change that part to make it work for VO.
Comment 7 Nan Wang 2016-03-14 18:44:52 PDT
Comment on attachment 274039 [details]
patch

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

>>>>> Source/WebCore/accessibility/AccessibilityObject.cpp:1881
>>>>> +    // If empty or missing, we should walk the parent chain to find the current status.
>>>> 
>>>> should we do this for every object, or should VoiceOver be responsible for checking the parent chain for the presence of aria-current?
>>>> i feel like the later might be more performant and more accurate.
>>> 
>>> I did have the same concern when implementing this. I'll try the VO approach too, but even in that case, we still need to check every object, right?
>> 
>> true but VO only has to do it when it needs to output the info. Does webkit end up calling this method for every object when its retrieved? or does it also only return this info when explicitly asked.
>> 
>> I feel its also slightly more accurate to only expose this on the actual element that does expose it.
> 
> Webkit is calling this method for every object, but only expose the attribute when result is not false. I think I should change that part to make it work for VO.

Oh never mind, that should work too.
Comment 8 Nan Wang 2016-03-15 19:26:14 PDT
Created attachment 274164 [details]
patch

Added aria-current to the list of global ARIA attributes.
Leave other logics to the VoiceOver side.
Comment 9 Build Bot 2016-03-15 20:32:35 PDT
Comment on attachment 274164 [details]
patch

Attachment 274164 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/986587

New failing tests:
transitions/default-timing-function.html
Comment 10 Build Bot 2016-03-15 20:32:40 PDT
Created attachment 274166 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 WebKit Commit Bot 2016-03-16 14:49:12 PDT
Comment on attachment 274164 [details]
patch

Clearing flags on attachment: 274164

Committed r198303: <http://trac.webkit.org/changeset/198303>
Comment 12 WebKit Commit Bot 2016-03-16 14:49:19 PDT
All reviewed patches have been landed.  Closing bug.