Bug 120372 – AX: AXValue/AXMenuItemMarkChar not defined for menuitemradio (should be false (0) with aria-checked=mixed | undefined)

• Component: Accessibility
• Status: RESOLVED FIXED

[Jon Gunderson]

According to the ARIA specification when ARIA role=menuitemradio has a aria-checked="mixed" property the AXvalue should be exposed and have the value=0

ARIA Test Case 344:
https://www.w3.org/WAI/PF/testharness/testcases/edit?testsuite_id=1&testcase_id=344 

ARIA Test Case 345:
https://www.w3.org/WAI/PF/testharness/testcases/edit?testsuite_id=1&testcase_id=345 

This is an important for requirement for ARIA 1.0 to move to recommendation status

[Radar WebKit Bug Importer]

<rdar://problem/14848449>

[chris fleizach]

Created attachment 210495 [details]
patch

[Jon Gunderson]

Public test files:

Test File for ARIA Test Case 344:
https://dvcs.w3.org/hg/pfwg/raw-file/default/ARIA/1.0/tests/test-files/roles-properties-supported-inherited/roles-properties-supported-inherited-menuitemradio-aria-checked-mixed.html

Test File for ARIA Test Case 345:
https://dvcs.w3.org/hg/pfwg/raw-file/default/ARIA/1.0/tests/test-files/roles-properties-supported-inherited/roles-properties-supported-inherited-menuitemradio-aria-checked-undefined.html

[James Craig]

Hold off on the commit-queue+ on this one for now. ARIA spec has a weird rule related to this one that the WG is discussing at the moment. It explicitly says to ignored the mixed state one menuitems for 1.0, but it's possible this will change in 1.1. The included patch may need to be updated depending on the outcome.

[James Craig]

Follow-up from last comment: 

Here's the text, but I'm not yet sure I agree with this behavior.

"The mixed value is not supported on radio or menuitemradio or any element that inherits from these in the taxonomy, and user agents MUST treat a mixed value as equivalent to false for those roles."

Cite: http://www.w3.org/WAI/PF/aria/complete#aria-checked

[chris fleizach]

(In reply to comment #5)
> Follow-up from last comment: 
> 
> Here's the text, but I'm not yet sure I agree with this behavior.
> 
> "The mixed value is not supported on radio or menuitemradio or any element that inherits from these in the taxonomy, and user agents MUST treat a mixed value as equivalent to false for those roles."
> 
> Cite: http://www.w3.org/WAI/PF/aria/complete#aria-checked

So is this assessment final then for ARIA 1.0?

[James Craig]

Yes. Short of a major event, 1.0 is locked down. If you like, you can commit an updated patch matching the speced 1.0 behavior… Just be aware that it may change in the future.

[James Craig]

Ah, I see the discrepancy now. "mixed" is supported on both checkbox and menuitemcheckbox, but not radio or menuitemradio. Given that clarification, I think that behavior is correct and this is unlikely to change in ARIA 1.1. I was probably just misreading the spec.

[chris fleizach]

(In reply to comment #8)
> Ah, I see the discrepancy now. "mixed" is supported on both checkbox and menuitemcheckbox, but not radio or menuitemradio. Given that clarification, I think that behavior is correct and this is unlikely to change in ARIA 1.1. I was probably just misreading the spec.

So it sounds like we still need to fix this bug, but we can't return a mixed state for some items..

[James Craig]

(In reply to comment #9)

> So it sounds like we still need to fix this bug, but we can't return a mixed state for some items..

If I'm understanding you correctly, you're saying the above patch is valid because it changes AXValue from undefined, to 0 (false), 1 (true), or 2 (mixed), which accounts for part of the fix, but the bug report was specifically about the mixed state for menuitemradio (and radio) should be equivalent to false. So the patch is incomplete.

[James Craig]

Also, your patch is missing AXMenuItemMarkChar ✓, which is how we'd expect to see the "checked" state on menuitemradio and menuitemcheckbox

[chris fleizach]

(In reply to comment #11)
> Also, your patch is missing AXMenuItemMarkChar ✓, which is how we'd expect to see the "checked" state on menuitemradio and menuitemcheckbox

This should already be working correctly

[chris fleizach]

Created attachment 211104 [details]
patch

New patch to make sure that radio and menu item radio do NOT expose mixed states

[Mario Sanchez Prada]

Comment on attachment 211104 [details]
patch

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

Looks good to me. Just fix the path for the JS resources before landing, and maybe consider my comment about the duplicate line.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1212
>          [tempArray addObject:NSAccessibilitySelectedAttribute];
> +        [tempArray addObject:NSAccessibilitySelectedAttribute];

Are you adding this duplicate line intentionally, or is it a mistake?

> LayoutTests/platform/mac/accessibility/menu-item-values.html:4
> +<script src="../../../fast/js/resources/js-test-pre.js"></script>

This path should be ../../../resources/js-test-pre.js now, as those JS resources have been moved there along with r155411

> LayoutTests/platform/mac/accessibility/menu-item-values.html:46
> +<script src="../../../fast/js/resources/js-test-post.js"></script>

Same here, but for ../../../resources/js-test-post.js

[chris fleizach]

Comment on attachment 211104 [details]
patch

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

>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1212
>> +        [tempArray addObject:NSAccessibilitySelectedAttribute];
> 
> Are you adding this duplicate line intentionally, or is it a mistake?

Yea this looks wrong, i'll fix that

[chris fleizach]

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