477975 - Expose action on HTML5 indeterminate checkboxes

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

Question: What should this be called? There are two scenarios:

- An undeterminate checkbox, when toggled, is set to next be fully checked.
- An indeterminate checkbox, when toggled, is next set to be fully unchecked.

We have no way of knowing what the next state will be, since this fully depends on the page author's JavaScript.

My initial suggestion would be to call this action "toggle", since this is neutral to what will actually happen.

Other suggestions?

Comment 1

[Willie Walker]

My main opinion here is to remain consistent with existing solutions.  The GTK+ check boxes and toggle buttons all expose a "click" action as the default (i.e., the first) action.

There is one inconsistency in GTK+, however, and that is with table cells that also happen to be check boxes.  That is, it is a single object that is of role table cell rather than being a table cell that has a check box as a child.  In these cases, we end up seeing an action of "toggle" exposed and is, in fact, the method Orca uses to infer that a table cell is a check  box.

Comment 2

[Aaron Leventhal]

"Cycle"

We already have that for items that cycle through 3 or more possibilities in tree grids.

Comment 3

[James Teh [:Jamie]]

Either "cycle" or "toggle" is fine with me.

Personally, I've always had a problem with "click", as it generally relates to using the mouse; I prefer "activate", "select" or the like. However, most would argue I'm being unnecessarily pedantic as usual. :)

Comment 4

[Marco Zehe (:MarcoZ)]

Attachment: Patch

1. Expose an action name of "cycle" on both aria-checked="mixed" attributed and those HTML5 controls that have indeterminate=true set via JS.
2. Renamed test_nsIAccessible_actions.html to test_actions_aria.html, since all that is tested in here has ARIA roles.
3. Renamed test_nsIAccessible_actions.xul to test_actions.xul and reenabled the file.
4. added test_actions_inputs.html to test some regular HTML:input elements and html:button.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 365924 [details] [diff] [review]
Patch

>diff --git a/accessible/src/base/nsAccessible.cpp b/accessible/src/base/nsAccessible.cpp

>    case eCheckUncheckAction:
>-     if (states & nsIAccessibleStates::STATE_CHECKED)
>+     if (states & nsIAccessibleStates::STATE_MIXED)
>+       aName.AssignLiteral("cycle");
>+     else if (states & nsIAccessibleStates::STATE_CHECKED)
>        aName.AssignLiteral("uncheck");
>      else
>        aName.AssignLiteral("check");
>      return NS_OK;

Nit: I would change the ordering and check for STATE_CHECKED, then STATE_UNCHECKED, and then default to "cycle" since that's is least likely, and do the same in GetActionName.

The rest looks good.

r=me

Comment 6

[Marco Zehe (:MarcoZ)]

I did make that change, but am hesitant to push this patch. When running the whole test suite locally, I get the tests running endlessly. No failures, but it doesn't run all the way through, but starts over at some point. Two consistent breaks are these:

[...]
793 INFO Running chrome://mochikit/content/a11y/accessible/test_nsIAccessible_applicationAccessible.html...
794 INFO TEST-PASS | chrome://mochikit/content/a11y/accessible/test_nsIAccessible_applicationAccessible.html | wrong application accessible name
796 INFO Running chrome://mochikit/content/a11y/accessible/test_nsIAccessible_comboboxes.xul...
0 INFO SimpleTest START
1 INFO Running chrome://mochikit/content/a11y/accessible/test_actions.xul...
[...]

Comment 7

[Marco Zehe (:MarcoZ)]

Alex, do you have an idea why my patch could be causing an infinite test run loop?

Comment 8

[Marco Zehe (:MarcoZ)]

Pushed in changeset:
http://hg.mozilla.org/mozilla-central/rev/d3ee07a4c551

I had to revert the review comment address: See bug 482258. Right now, aria-checked="mixed" causes STATE_CHECKED to be set on the control, which is wrong.

I also had to disable test_nsIAccessible_comboboxes.xul. The additional tests from this bug uncover a problem that has the potential to cause random failures with that file, among htem endless looping through the test suite.

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #8)

> I also had to disable test_nsIAccessible_comboboxes.xul. The additional tests
> from this bug uncover a problem that has the potential to cause random failures
> with that file, among htem endless looping through the test suite.

Marco, sorry if I'm nitpicker but technically it's called regression (new code prevents old code to work successfully).

Comment 10

[Michael Ventnor]

(In reply to comment #5)
> Nit: I would change the ordering and check for STATE_CHECKED, then
> STATE_UNCHECKED, and then default to "cycle" since that's is least likely, and
> do the same in GetActionName.

Not good. According to HTML5 (and our implementation) indeterminate doesn't affect the "checked" state, it's a separate property and event state. For it to work as expected it must be looked at before the checked property.

Comment 11

[Marco Zehe (:MarcoZ)]

(In reply to comment #10)
> Not good. According to HTML5 (and our implementation) indeterminate doesn't
> affect the "checked" state, it's a separate property and event state. For it to
> work as expected it must be looked at before the checked property.

Then this one conflicts with the ARIA spec for checkboxes:
http://www.w3.org/TR/wai-aria/#checkbox
which specifically says it's true, false, OR mixed, which is also what is being seen in Windows or other operating systems with so-called tri-state checkboxes. Also from what I understand, our implementation represents an indeterminate checkbox with something that looks like a partially checked checkbox, which is neither fully checked nor fully unchecked. This is what the "mixed" state in a11y is trying to communicate.

Question: Is there a visual difference between
<input type="checkbox" checked="true"/> and its indeterminate DOM property set to true, and
<input type="checkbox"/> and its indeterminate property set to true?

Comment 12

[Michael Ventnor]

No. Indeterminate always overrides the checked state in terms of visuals. That's why I think it's a problem for indeterminate to be read after checked.

Comment 13

[Marco Zehe (:MarcoZ)]

(In reply to comment #12)
> No. Indeterminate always overrides the checked state in terms of visuals.
> That's why I think it's a problem for indeterminate to be read after checked.

Our fix for the "mixed" state from bug 477572 takes care of this already. It checks to see if the indeterminate property is set, and if so, does not even check for "checked" state anymore.Since bug 482258, we even have tests in place to make sure a checkbox with mixed state is never checked at the same time, and vice versa. This particular bug is just about exposing the action names, which gets determined after the state checking has already been concluded.