WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
123889
[ATK] "Indeterminate" state and state changes in tri-state checkboxes should be exposed to ATs
https://bugs.webkit.org/show_bug.cgi?id=123889
Summary
[ATK] "Indeterminate" state and state changes in tri-state checkboxes should ...
Joanmarie Diggs
Reported
2013-11-06 05:16:12 PST
Created
attachment 216169
[details]
accessible-event listener Steps to reproduce: 1. (Re)Load
http://test.cita.illinois.edu/aria/checkbox/checkbox2.php
2. Launch the attached accessible-event listener in a terminal 3. Tab to All Condiments (which initially is partially checked) 4. Tab to Lettuce 5. Press Space (All Condiments will lose partially checked state) 6. Press Space (All Condiments will gain partially checked state) Expected results: $ ./indeterminate.py [check box | All condiments] has indeterminate state: True [check box | Lettuce] has indeterminate state: False [check box | All condiments] indeterminate state changed to: False [check box | All condiments] indeterminate state changed to: True Actual results with current WebKitGtk MiniBrowser: $ ./indeterminate.py [check box | All condiments] has indeterminate state: False [check box | Lettuce] has indeterminate state: False
Attachments
accessible-event listener
(666 bytes, text/x-python)
2013-11-06 05:16 PST
,
Joanmarie Diggs
no flags
Details
patch
(2.71 KB, patch)
2014-01-15 06:00 PST
,
Krzysztof Czech
no flags
Details
Formatted Diff
Diff
patch
(
deleted
)
2014-02-06 09:16 PST
,
Krzysztof Czech
no flags
Details
Formatted Diff
Diff
patch
(13.79 KB, patch)
2014-02-13 06:23 PST
,
Krzysztof Czech
no flags
Details
Formatted Diff
Diff
patch
(13.86 KB, patch)
2014-02-13 06:33 PST
,
Krzysztof Czech
no flags
Details
Formatted Diff
Diff
patch
(8.47 KB, patch)
2014-02-14 03:01 PST
,
Krzysztof Czech
cfleizach
: review-
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
(486.21 KB, application/zip)
2014-02-14 04:23 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
(503.13 KB, application/zip)
2014-02-14 04:53 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
(485.83 KB, application/zip)
2014-02-14 05:21 PST
,
Build Bot
no flags
Details
Modified version of the UIUC checkbox 2 test
(7.17 KB, text/html)
2014-02-14 20:29 PST
,
Joanmarie Diggs
no flags
Details
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2013-11-06 05:16:49 PST
<
rdar://problem/15403059
>
Krzysztof Czech
Comment 2
2014-01-15 06:00:48 PST
Created
attachment 221262
[details]
patch
Krzysztof Czech
Comment 3
2014-01-15 06:09:59 PST
I proposed a patch for exposing tri-state checkboxes. It's a kind of a draft not a final version. I think it would be nice to have a layout test as well. Basically I'd like to hear an opinion about my approach. WebKitGtk MiniBrowser works as expected.
Mario Sanchez Prada
Comment 4
2014-01-15 07:04:51 PST
Comment on
attachment 221262
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=221262&action=review
I like the approach. I only have a couple of comments
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:188 > + if (coreObject->roleValue() != GroupRole) > + axGroup = coreObject->parentObjectUnignored();
Not sure whether this should be a while instead of an if. Would it be possible that the parent with the GroupRole role is not the immediate unignored parent for the object which received the notification?
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:190 > + AccessibilityObject::AccessibilityChildrenVector children = axGroup->children();
As per recent findings, you should not be copying the list of children here. use const auto& children = axGroup->children() instead.
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:201 > + // Get tri-state head object
Missing period at the end
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:239 > + notifyCheckedStateChanged(coreObject); > + notifyCheckedStateIndeterminate(coreObject);
I would probably call notifyCheckedStateIndeterminate() from inside notifyCheckedStateChanged(), or even combine both functions into a simple one because in the end, from this specific point, it's all about notifying the "checked state". And in this context, the "indeterminate" checked state seems to be IMHO an internal detail that should not be exposed here.
Krzysztof Czech
Comment 5
2014-01-16 02:52:40 PST
(In reply to
comment #4
)
> (From update of
attachment 221262
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=221262&action=review
> > I like the approach. I only have a couple of comments > > > Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:188 > > + if (coreObject->roleValue() != GroupRole) > > + axGroup = coreObject->parentObjectUnignored(); > > Not sure whether this should be a while instead of an if. Would it be possible that the parent with the GroupRole role is not the immediate unignored parent for the object which received the notification?
I also have the same doubt and couple of others, regarding this peace of code. I guess GroupRole objects may not be an immediate parent. I can imagine for example other element between, then tri-state object would not be the first(0) element. I'm also wondering whether there might be something different than GroupRole element. Basically in this specific test case GroupRole groups those checkboxes and this is obvious for me, but is it possible that other element could do the same thing or maybe checkboxes could not be grouped at all. Those are things that bothers me.
> > > Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:190 > > + AccessibilityObject::AccessibilityChildrenVector children = axGroup->children(); > > As per recent findings, you should not be copying the list of children here. use const auto& children = axGroup->children() instead.
Thanks.
> > > Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:201 > > + // Get tri-state head object > > Missing period at the end > > > Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:239 > > + notifyCheckedStateChanged(coreObject); > > + notifyCheckedStateIndeterminate(coreObject); > > I would probably call notifyCheckedStateIndeterminate() from inside notifyCheckedStateChanged(), or even combine both functions into a simple one because in the end, from this specific point, it's all about notifying the "checked state". And in this context, the "indeterminate" checked state seems to be IMHO an internal detail that should not be exposed here.
Yes, you are right.
Krzysztof Czech
Comment 6
2014-02-06 09:16:14 PST
Created
attachment 223338
[details]
patch
chris fleizach
Comment 7
2014-02-06 09:48:58 PST
Comment on
attachment 223338
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=223338&action=review
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:203 > + // WebKit notifies it as many times as children changes its states.
I think we should try to resolve the issue of multiple updates being posted rather than storing a boolean value and a list of tri state objects in every AXObject. That seems to paper over the problem which may affect other cases as well as this one
Krzysztof Czech
Comment 8
2014-02-13 06:22:11 PST
> > Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:203 > > + // WebKit notifies it as many times as children changes its states. > > I think we should try to resolve the issue of multiple updates being posted rather than storing a boolean value and a list of tri state objects in every AXObject. That seems to paper over the problem which may affect other cases as well as this one
Basically those notifications from children are triggered by a tri-state object when its state is changed, so this is a proper behavior. The thing was that webkit notified them as they came from a tri-state object which was wrong. This happened because child elements were not accessible. Regarding this boolean value. I guess there is no other way to know that particular element is a tri-state objects. Well there's an option to check the aria-checked attribute but when the state is different than mixed then it's not possible. So my idea is to store this information in AccessibilityObject every time I find a proper element with a mixed state. Having this value, I'm able to send a signal to ATs when this state is set or not.
Krzysztof Czech
Comment 9
2014-02-13 06:23:00 PST
Created
attachment 224064
[details]
patch
WebKit Commit Bot
Comment 10
2014-02-13 06:25:03 PST
Attachment 224064
[details]
did not pass style-queue: ERROR: Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:210: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Krzysztof Czech
Comment 11
2014-02-13 06:33:08 PST
Created
attachment 224065
[details]
patch
chris fleizach
Comment 12
2014-02-13 09:25:56 PST
Comment on
attachment 224065
[details]
patch I don't support this change. I think this can be done without storing a bool value in every object and without making a list of all possible tri-state objects that needs to be checked. It seems like you just need to know if a radio button's aria-value was mixed -> not mixed, or not mixed -> mixed That seems like it's possible to know already by monitoring attribute changes
Krzysztof Czech
Comment 13
2014-02-14 00:46:26 PST
(In reply to
comment #12
)
> (From update of
attachment 224065
[details]
) > I don't support this change. I think this can be done without storing a bool value in every object and without making a list of all possible tri-state objects that needs to be checked. > It seems like you just need to know if a radio button's aria-value was mixed -> not mixed, or not mixed -> mixed > That seems like it's possible to know already by monitoring attribute changes
Thanks Chris for your comments. I guess I tried to complicate things a bit. I found some valuable information in aria-practices that put some light on things that I wasn't really sure.
Krzysztof Czech
Comment 14
2014-02-14 03:01:50 PST
Created
attachment 224190
[details]
patch
Krzysztof Czech
Comment 15
2014-02-14 03:16:43 PST
(In reply to
comment #14
)
> Created an attachment (id=224190) [details] > patch
w3 says (
http://www.w3.org/TR/2013/WD-wai-aria-practices-20130307/
) that checkboxes that have logical grouping should be children of a DOM element with group role. Related to previous Mario's comments, this seems that an immediate parent of the children that received notification is one with group role. My only concern is that whether we can treat the first element of a group as tri-state object, but I could not imagine other place, so it likely to be the first child.
Build Bot
Comment 16
2014-02-14 04:23:28 PST
Comment on
attachment 224190
[details]
patch
Attachment 224190
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/5956747923554304
New failing tests: accessibility/aria-checkbox-sends-indeterminate-notification.html
Build Bot
Comment 17
2014-02-14 04:23:31 PST
Created
attachment 224198
[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 18
2014-02-14 04:53:45 PST
Comment on
attachment 224190
[details]
patch
Attachment 224190
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/4908985148768256
New failing tests: accessibility/aria-checkbox-sends-indeterminate-notification.html
Build Bot
Comment 19
2014-02-14 04:53:49 PST
Created
attachment 224200
[details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Build Bot
Comment 20
2014-02-14 05:21:37 PST
Comment on
attachment 224190
[details]
patch
Attachment 224190
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/6321002623533056
New failing tests: accessibility/aria-checkbox-sends-indeterminate-notification.html
Build Bot
Comment 21
2014-02-14 05:21:43 PST
Created
attachment 224205
[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
Mario Sanchez Prada
Comment 22
2014-02-14 05:24:11 PST
(In reply to
comment #15
)
> (In reply to
comment #14
) > > Created an attachment (id=224190) [details] [details] > > patch > > w3 says (
http://www.w3.org/TR/2013/WD-wai-aria-practices-20130307/
) that checkboxes that have logical grouping should be children of a DOM element with group role. > > Related to previous Mario's comments, this seems that an immediate parent of the children that received notification is one with group role. > > My only concern is that whether we can treat the first element of a group as > tri-state object, but I could not imagine other place, so it likely to be the > first child.
I also find strange this thing of emitting the signal always from the first element (what happens if the state has actually changed over one of the others?). In GTK you can get that signal emitted by any checkbox in the group, I think, so my understanding is that this scenario should be more or less similar. Perhaps Joanie can drop some light in here. After all, she reported the bug and probably knows better what Orca would actually expect from its side
chris fleizach
Comment 23
2014-02-14 09:02:40 PST
Comment on
attachment 224190
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=224190&action=review
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:186 > + AccessibilityObject* currentObject = coreObject;
probably don't need to make a new currentObject here. you could rename the incoming parameter to just "object" and use that
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:190 > + const AccessibilityObject::AccessibilityChildrenVector& children = currentObject->children();
const auto& children
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:212 > + AtkObject* atkObject = accessibilityTriStateObject ? accessibilityTriStateObject->wrapper() : nullptr;
i don't think you need to check if accessibilityTriStateObject == nil here because you already did that on line 205
> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:213 > + if (atkObject) {
you can probably write this as if (AtkObject* atkObject = axTriState->wrapper()) {
Joanmarie Diggs
Comment 24
2014-02-14 20:29:37 PST
Created
attachment 224278
[details]
Modified version of the UIUC checkbox 2 test (Discussion to follow)
Joanmarie Diggs
Comment 25
2014-02-14 21:25:15 PST
(In reply to
comment #15
)
> w3 says (
http://www.w3.org/TR/2013/WD-wai-aria-practices-20130307/
) that checkboxes that have logical grouping should be children of a DOM element with group role.
Indeed it does. If you have a bunch of checkboxes all related to foo, grouping those checkboxes together into a "foo" group makes sense. BUT I don't think that's relevant to the problem at hand. 1. Just because you have a "foo" group of checkboxes does not necessarily mean you have an associated "toggle all foos" uber tri-state checkbox. 2. If you happen to have an associated "toggle all foos" uber tri-state checkbox, there is no specification (that I have found) indicating where that uber checkbox must go. (Nor should there be IMHO.)
> My only concern is that whether we can treat the first element of a group as tri-state object, but I could not imagine other place
Your concern is valid. :) I created a local/stand-alone copy of the UIUC checkbox 2 test and added cheese. :) The "all cheeses" checkbox is the last child; not the first. Firefox does the right thing; your patch does not. Related aside #1: When I try to interact with my newly-attached test case as an attachment (i.e. link followed in bugzilla), the checkboxes are not interactive. But if I download the test case to my system and open it as a local file, it works as expected. <insert shrug here> Getting back to the topic at hand, I have what I hope is some good news for you: You should NOT have to do heuristics to figure out where the tri-state object is. When a checkbox-roled element (e.g. Lettuce) changes its state, it is up to the web developer to set the state of that checkbox-roled element. If that state change in turn causes some other checkbox (e.g. All condiments) to also change state, then it is up to the web developer to also set the state of that other checkbox-roled element (i.e. All condiments). It ain't your responsibility to sort out this mess, in other words. :) You should "just have" the checkbox and the state change. Just pass that information along via the platform accessibility API. :) Related aside #2: It appears that the UIUC checkbox 2 example sets the "mixed" state even when the mixed state has not changed. See adjCheckedCount(). I did not fix this in my updated version of the UIUC example. Apparently Gecko is filtering this non-change out and only emitting state-changed:indeterminate signals if the indeterminate state really has changed. While that is appreciated noise reduction, it is not clear to me whose bug/responsibility it is to only emit state-changed signals when the state has actually changed. I'm leaning towards this being an author error.... Anyhoo, let's separate that out from the originally filed bug here. My question to you is: Using the modified (and downloaded) new test case, are you seeing WebCore Accessibility events for checkboxes which you can pass along unmodified via ATK? If the answer is "yes", please do so. :) If not, please elaborate.
Krzysztof Czech
Comment 26
2014-02-17 03:03:45 PST
(In reply to
comment #25
)
> (In reply to
comment #15
) > > > w3 says (
http://www.w3.org/TR/2013/WD-wai-aria-practices-20130307/
) that checkboxes that have logical grouping should be children of a DOM element with group role. > > Indeed it does. If you have a bunch of checkboxes all related to foo, grouping those checkboxes together into a "foo" group makes sense. BUT I don't think that's relevant to the problem at hand. > > 1. Just because you have a "foo" group of checkboxes does not necessarily mean you have an associated "toggle all foos" uber tri-state checkbox.
> > 2. If you happen to have an associated "toggle all foos" uber tri-state checkbox, there is no specification (that I have found) indicating where that uber checkbox must go. (Nor should there be IMHO.)
Thank you for your valuable clarification. It seems that there could be more then one tri-state object as well as other checkboxes in a group that are just standalone elements and not associated in any relation.
> Getting back to the topic at hand, I have what I hope is some good news for you: You should NOT have to do heuristics to figure out where the tri-state object is. When a checkbox-roled element (e.g. Lettuce) changes its state, it is up to the web developer to set the state of that checkbox-roled element. If that state change in turn causes some other checkbox (e.g. All condiments) to also change state, then it is up to the web developer to also set the state of that other checkbox-roled element (i.e. All condiments). It ain't your responsibility to sort out this mess, in other words. :) You should "just have" the checkbox and the state change. Just pass that information along via the platform accessibility API. :)
This is right, as soon as state-change comes I'm able to send proper notification via platform accessibility API for example (not-indeterminate->indeterminate) but when (indeterminate->not-indeterminate) comes without knowledge about this specific tri-state checkbox I guess, it's hard to send a notification that I think one should be associated to this tri-state element that (indeterminate is false). I'm referring to this: [check box | All condiments] indeterminate state changed to: False [check box | All condiments] indeterminate state changed to: True Existing API only provides the information that state has changed. There is no info about the previous state of the element. I guess some little heuristic should be done here to find this element. What do you think ?
> > Related aside #2: It appears that the UIUC checkbox 2 example sets the "mixed" state even when the mixed state has not changed. See adjCheckedCount(). I did not fix this in my updated version of the UIUC example. Apparently Gecko is filtering this non-change out and only emitting state-changed:indeterminate signals if the indeterminate state really has changed. While that is appreciated noise reduction, it is not clear to me whose bug/responsibility it is to only emit state-changed signals when the state has actually changed. I'm leaning towards this being an author error.... Anyhoo, let's separate that out from the originally filed bug here. > > My question to you is: Using the modified (and downloaded) new test case, are you seeing WebCore Accessibility events for checkboxes which you can pass along unmodified via ATK? If the answer is "yes", please do so. :) If not, please elaborate.
I'm able to see Accessibility events for checkboxes with new test cases. I could pass them along with an available ATK API such as: atk_object_notify_state_change(..., ATK_STATE_CHACKED/ATK_STATE_INDETERMINATE,...) Still bother me this test case I mentioned before.
Joanmarie Diggs
Comment 27
2014-02-17 03:52:54 PST
(In reply to
comment #26
)
> Existing API only provides the information that state has changed. There is no info about the previous state of the element. I guess some little heuristic should be done here to find this element. What do you think ?
If you literally mean "find" that element, I think that's a bad idea -- and an unnecessary one: Given a tri-state checkbox foo, any time foo changes state -- for whatever reason, including bar changing state -- foo itself should notify WebKit that its state has changed. In other words, if bar changes state you should not try to find foo. Foo should come to you with its own web-app-emitted signal. If, on the other hand, all you are told is "foo changed state to checked/unchecked" and you have no way of knowing if foo might be a tri-state that just lost state mixed/indeterminate.... a) That sucks. ;) b) Determining that foo is tri-state *might* require a heuristic. Make sense?
Krzysztof Czech
Comment 28
2014-02-17 05:14:17 PST
(In reply to
comment #27
)
> (In reply to
comment #26
) > > > Existing API only provides the information that state has changed. There is no info about the previous state of the element. I guess some little heuristic should be done here to find this element. What do you think ? > > If you literally mean "find" that element, I think that's a bad idea -- and an unnecessary one: Given a tri-state checkbox foo, any time foo changes state -- for whatever reason, including bar changing state -- foo itself should notify WebKit that its state has changed. In other words, if bar changes state you should not try to find foo. Foo should come to you with its own web-app-emitted signal.
Exactly, this is how it works. Foo posted its own signal and then to find out what state it has, depends whether Foo is aria checkbox aria-checked attribute should be read (true/false/mixed).
> > If, on the other hand, all you are told is "foo changed state to checked/unchecked" and you have no way of knowing if foo might be a tri-state that just lost state mixed/indeterminate.... a) That sucks. ;) b) Determining that foo is tri-state *might* require a heuristic. > > Make sense?
Yes, it makes sense. This is exactly the case that bothers me, how to know if foo might a tri-state object when lost its state mixed/indeterminate. I guess the only way of knowing mixed/indeterminate is by reading aria-checked attribute that I mentioned before, so monitoring changes of aria-checked attribute will not solve this issue.
Steve Faulkner
Comment 29
2014-07-01 05:15:03 PDT
I have been checking support for HTML indeterminate IDL attribute test file
http://stevefaulkner.github.io/html-mapping-tests/browser-tests/checkbox-states.html
as part of testing the implementation of acc layer in browsers:
http://stevefaulkner.github.io/html-mapping-tests/
I have found that <input type=checkbox> indeterminate= true in firefox and Chrome on windows exposes as the MSAA mixed state. Having checked webkit latest build on Mac have found that while aria-checked=mixed is exposed as value=2, indeterminate= true is not. Is this something I should file a new bug on?
chris fleizach
Comment 30
2014-07-01 08:51:46 PDT
(In reply to
comment #29
)
> I have been checking support for HTML indeterminate IDL attribute > test file
http://stevefaulkner.github.io/html-mapping-tests/browser-tests/checkbox-states.html
> > as part of testing the implementation of acc layer in browsers:
http://stevefaulkner.github.io/html-mapping-tests/
> > I have found that <input type=checkbox> indeterminate= true in firefox and Chrome on windows exposes as the MSAA mixed state. Having checked webkit latest build on Mac have found that while aria-checked=mixed is exposed as value=2, indeterminate= true is not. > > Is this something I should file a new bug on?
Yes please
Ferdinand Prantl
Comment 31
2022-10-05 10:40:29 PDT
May I asked about the state of this issue, please? Safari is the only browser, which doesn't include the state "mixed" in the accessibility tree, when the property `indeterminate` is set to true on an input element of the type "checkbox". Both Chrome and Firefox do it and a screen reader can read the checkbox state properly. Safari doesn't and it causes more work for application developers, who have to support Safari by a custom checkbox element. Can WebKit work the same like other browsers, which use two boolean properties to control the state in the accessibility tree? indeterminate = true => "mixed" otherwise => checked ? "checked" : "unchecked"
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug