Closed Bug 388191 Opened 12 years ago Closed 3 years ago

No state-changed events for Thunderbird's Message Filters dialog checkboxes

Categories

(Thunderbird :: Disability Access, defect)

x86
Linux
defect
Not set

Tracking

(thunderbird52 fixed, thunderbird53 fixed, thunderbird54 fixed)

RESOLVED FIXED
Thunderbird 54.0
Tracking Status
thunderbird52 --- fixed
thunderbird53 --- fixed
thunderbird54 --- fixed

People

(Reporter: jdiggs, Assigned: Paenglab)

References

(Blocks 2 open bugs)

Details

(Keywords: access, Whiteboard: [bk1] orca:high)

Attachments

(1 file, 1 obsolete file)

Steps to reproduce:

1. Create a message filter in Thunderbird.
2. Launch Accerciser, turn on event monitoring.
3. Get into the Message Filters dialog, arrow to the desired filter and use the space bar to toggle its state.

Expected results:  A state-changed:checked event would be issued each time the checkbox's state is toggled.

Actual results:  No state-changed:checked events are issued when the checkbox's state is toggled.
Blocks: tbird3access
Whiteboard: orca:major
Whiteboard: orca:major → orca:high
Mass un-assigning bugs assigned to Aaron.
Assignee: aaronleventhal → nobody
I'm pretty sure this UI has been re-written. Could someone re-confirm the bug?
Whiteboard: orca:high → [bk1] orca:high
(In reply to David Bolter [:davidb] from comment #2)
> I'm pretty sure this UI has been re-written. Could someone re-confirm the
> bug?

ace, can you confirm?
I don't know what Accerciser. But I am not sure what the report actually means.

But I know there was bug 481682 which caused the filter state not really be changed. I am not sure if that error influenced the events at the widget level. It is quite possible as due to the error (I do not remember if it threw an exception) maybe toggleFilter was never called, which should call .setAttribute("enabled", aFilter.enabled), which may be the event the reporter wants.

This definitely needs retesting from the original reporter on TB15+.
Component: Disability Access APIs → Disability Access
Depends on: 481682
Product: Core → Thunderbird
QA Contact: accessibility-apis → disability-access
So can anybody try it out?
Just did in 15.0a1 (2012-05-28) - Fedora 18/Rawhide.

Still no events. Interestingly enough, I don't even see the checkbox in the accessible hierarchy. :-/
Maybe those aren't true checkboxes, but some images that just simulate a checkbox. Can you try the addressbook whitelist in account settings -> Junk settings? Do those "checkboxes" have the same problem?
No longer blocks: tbird3access
Those checkboxes do not have the same problem: They both show up in the accessible hierarchy in Accerciser and emit the expected object:state-changed:checked events.
I'm experiencing the issue like this in Thunderbird 38.2.0.
I'm not checking with Accessizer, the screen readers here, JAWS 16 and NVDA 2015.3, does not give any feedback for pressing space there. I don't know the state is actually changing with the keystroke.
Duplicate of this bug: 1333847
Abdelkrim, the build was successful. Please can you test the build from https://archive.mozilla.org/pub/thunderbird/try-builds/richard.marti@gmail.com-e7893319eb430a5840d0be81e20f34804b88e594/try-comm-central-win32/ and write here if this approach works?
Flags: needinfo?(abdelkrim.bensaid)
Hello Richard,

First of all, I wanted to thank you for this work.

I just tested with NVDA.

I notice that when the item is selected in the filter list, its status is still not reported.

However, the novelty is that the second child of this element is indeed a checkbox, but it does not have the focus, so it is not recognized as a priority.

With JAWS, the problem is similar. Scripts can be used to correct this malfunction, but it might be better if they are corrected in Thunderbird itself.

Can you make it the element of the filter list that is directly recognized as a checkbox?
I thank you in advance.

Best regards.
I'll look if I can add a checked="true|false" to the top listitem like the addressbook whitelist from comment 8. Then my last changes aren't probably needed as they don't really help.
(In reply to Richard Marti (:Paenglab) from comment #15)
> I'll look if I can add a checked="true|false" to the top listitem like the
> addressbook whitelist from comment 8. Then my last changes aren't probably
> needed as they don't really help.

I'm not sure if I understand Abdelkrim's description right, but it may work to move the role and aria-checked from the listcell to the listitem element.
I don't think it's good to give the listitem the checkmark role as it is then not clear if this is now a listitem or a checkmark. I think it's better to use the the addressbook whitelist approach with checked="true|false".
aria-checked can be applied to certain roles only, if that's a listitem, then role='option' on it may do a trick. Also it seems there's a focus problem per comment #14: when an item is checked/unchecked, then it has to have the user focus. Where the DOM focus is?
(In reply to alexander :surkov from comment #18)
> Where the DOM focus is?

Hi Alexander,

When I view the innerHTML property of the list item with NVDA, using ISimpleDOMNode, I get the following:

<listcell xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" label="General-mozilla"/><listcell xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" class="listcell-iconic" role="checkbox" enabled="true" aria-checked="true"/>

With JAWS, I have not figured out how to do it.

Unless I'm mistaken, ISimpleDOM should be accessible by all screen readers, right?

Thank you.
Flags: needinfo?(abdelkrim.bensaid)
(In reply to Richard Marti (:Paenglab) from comment #15)
> I'll look if I can add a checked="true|false" to the top listitem like the
> addressbook whitelist from comment 8. Then my last changes aren't probably
> needed as they don't really help.



Hello, 

You can download below an extension that gives the enabled/disabled status of each selected item in the filter list and displays it in the status bar.

That is not what I really want, because I would prefer the state to be given
directly when one is on the item.

http://cyber25.free.fr/extensions/myfiltersstatus@accessibility.mail.xpi

I was able to create it by reading this tutorial: 

https://developer.mozilla.org/en-US/Add-ons/Thunderbird/Building_a_Thunderbird_extension

Richard, telle me if that can helps you.

I don't know if an aria property can be programmatically added in javascript with an oncommand event for example.

Thank you. 

Best regards.
Hi Richart,

I'm sorry to tell you that unfortunately, this new build brings nothing new.

With JAWS and NVDA, after several tests, we have returned to the starting, there is absolutely no indication that is announced regarding the checked/unchecked state of the selected item.

The previous build was a bit better, because when I positioned the cursor on the item, with NVDA, then navigating between the objects up to the first child and then to the next object, the checked / unchecked state was Well read by NVDA, and recognizable by the MSAA accState method.

Thank you.
Hello,

I just wanted to add that I used only the first link communicated in Richard's latest post.

The second link included trial versions, which I did not install.

I can only test build versionss.

Best regards.
(In reply to Abdelkrim BENSAID from comment #22)
> Hi Richart,
> 
> I'm sorry to tell you that unfortunately, this new build brings nothing new.
> 
> With JAWS and NVDA, after several tests, we have returned to the starting,
> there is absolutely no indication that is announced regarding the
> checked/unchecked state of the selected item.
> 
> The previous build was a bit better, because when I positioned the cursor on
> the item, with NVDA, then navigating between the objects up to the first
> child and then to the next object, the checked / unchecked state was Well
> read by NVDA, and recognizable by the MSAA accState method.

Strange, this should work like the addressbook whitelist in Account Manager Junk settings. Please can you check what you are seeing in Account Manager/Junk Settings and here the address Book list?
Hello, when I go into the account settings/Junk settings/, the list of address books is accessible in the form of check boxes.

But this is not the case for the list of filters.

A small detail, which might help you.

On the filter list, when I view the child object of the item, there is an object just to the right of this child object, the second listcell tag with the famous enabled="true" or enabled="false".

In the list of address books cited above, the item also contains a child, but there is no object to the right of this child object.

Thank you.
Hi Richard,

Finally, I think I found why screen readers did not recognize these check boxes.

The listitem tag is contained in a node hierarchy, not like the list of address books.

In the listitem tag contained in the listbox, we just have to add a role="checkbox" attribute and an aria-checked whose value will be the state of the checkbox.

The code below, allows me to have a checkbox checkable and uncheckable for each item:

var filterList = document.GetElementById("filterList");
var item = filterList.selectedItem;
var ariaRoleItem = document.createAttribute("role");
ariaRoleItem.value = "checkbox";
var ariaItemChecked = document.createAttribute("aria-checked");
item.setAttributeNode(ariaRoleItem);
ariaItemChecked.value = item.firstChild.nextSibling.getAttribute("enabled");
item.setAttributeNode(ariaItemChecked);

You have just to add these 2 attributes in the listitem tag in your last build and thanks for all.

Here is the correction of my extension "myfiltersstatus@accessibility.mail", which makes the checkboxes of the message filters easily accessible:

http://cyber25.free.fr/extensions/new/myfiltersstatus@accessibility.mail.xpi

Best regards.
Now that the builds are working again, the next try: https://archive.mozilla.org/pub/thunderbird/try-builds/richard.marti@gmail.com-363dcb1fa293cdfc76e62164a07d3d0d88fb7a74/try-comm-central-win32/

This should implement your approach.

One question with your normal Thunderbird can you read the filter label directly or is it only readable in child node? I'm asking this because I added the label directly to the listitem.
(In reply to Richard Marti (:Paenglab) from comment #27)

> One question with your normal Thunderbird can you read the filter label
> directly or is it only readable in child node? I'm asking this because I
> added the label directly to the listitem.

Hello Richard,

A big thank you for this last build, which perfectly corrects the bug.

All my screen readers now correctly interpret the checked / unchecked status of each item.

We can therefore consider this bug as resolved.

Concerning your question, it is better to put the label attribute directly on the listitem, rather than putting it in the childNode listcell.

In normal versions of Thunderbird, the name of the item was well recognized by the screen readers, despite the fact that the label was in a child node, but it is more logical that it is in the listitem itself .

However, if we put an aria attribute in a child node, the screen reader will not read it in the parent node, this is the problem we had in your first buid in this bug.

Best regards.
Attached patch accessFilter.patch (obsolete) — Splinter Review
Abdelkrim, thank you for the testing.

This patch adds the role and aria-checked to the listitem. I also added the label to the listitem. It was also needed to leave the label on the listcell as it doesn't inherit from listitem.

Do you know why it doesn't inherit like the addressbook whitelist listiten and his -cell does? I see no difference why this happens. The listcell of the addressbook whitelist listitem has xbl:inherits but the filterlis one not.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8831528 - Flags: review?(acelists)
(In reply to Richard Marti (:Paenglab) from comment #29)
> Abdelkrim, thank you for the testing.

> Do you know why it doesn't inherit like the addressbook whitelist listiten
> and his -cell does? I see no difference why this happens. The listcell of
> the addressbook whitelist listitem has xbl:inherits but the filterlis one
> not.

Hi Richard,

Thanks, I remain at your disposal for further tests.

In exploring the hierarchy of nodes, between the list of address books for junk settings and the list of filters, from the listbox tag, I observe the following differences:

1. The listitem tag for the list of address books is a single tag, while it is double in the filter list.

2. In the list of address books, this listitem tag has no children, whereas for the filter list it has two unique listcell tags as children.

That is probably why we did not get the same behavior.

Here is the DOM below, of each listbox tag, made on the last build.

1. for the address books in junk settings:

<listbox xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" id="whiteListAbURI" rows="5">
<listitem type="checkbox" class="listitem-iconic" label="carnet-d'adresses" value="moz-abmdbdirectory://impab.mab" checked="false" current="true"/>
<listitem type="checkbox" class="listitem-iconic" label="Collected Addresses" value="moz-abmdbdirectory://history.mab" checked="false"/>
<listitem type="checkbox" class="listitem-iconic" label="Personal Address Book" value="moz-abmdbdirectory://abook.mab" checked="true"/>
</listbox>

2. for the filterList:

<listbox id="filterList" flex="1" onselect="updateButtons();" seltype="multiple" onkeypress="onFilterListKeyPress(event);">
<listhead>
<listheader id="nameColumn" label="Filter Name" flex="1"/>
<listheader id="activeColumn" label="Enabled" minwidth="40px"/>
</listhead>
<listitem role="checkbox" label="NVDA-Dev" aria-checked="true">
<listcell label="NVDA-Dev"/>
<listcell class="listcell-iconic" enabled="true"/>
</listitem>
<listitem role="checkbox" label="NVDA" aria-checked="true">
<listcell label="NVDA"/>
<listcell class="listcell-iconic" enabled="true"/>
</listitem>
</listbox>

Best regards.
The only difference is the filterlist listitem has two childs (listcell) and the whiteListAbURI one. With DOMi I see attributes which aren't shown to screen readers.
(In reply to Richard Marti (:Paenglab) from comment #31)
> The only difference is the filterlist listitem has two childs (listcell) and
> the whiteListAbURI one. With DOMi I see attributes which aren't shown to
> screen readers.

Hi Richard,

Based on this assumption, we can deduce that when the listitem has more than one child, the screen reader does not recognize its type, whereas it recognizes it well if it has only one child.

For the moment, our test was only on the checkboxes, it should be extended on other types of element I think.

It does not really matter to our bug, since it has been fixed thanks to aria, but it is always interesting to know.
Comment on attachment 8831528 [details] [diff] [review]
accessFilter.patch

Review of attachment 8831528 [details] [diff] [review]:
-----------------------------------------------------------------

OK, this seems to work for me, the aria-checked attribute is properly toggled together with the real checkbox.
I just think we want to set the aria-* attributes after the real ones. That seems to be done in most of the existing occurrences in the tree.
First set the real thing like .label, then the aria-label, so that the readers do not speak something that is not yet on screen (but will appear in a moment).
Thanks.
Attachment #8831528 - Flags: review?(acelists) → review+
Reordered the lines.
Attachment #8831528 - Attachment is obsolete: true
Attachment #8837799 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/931e397b74a79ca2e29628e4d05b683564f2624b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 54.0
Comment on attachment 8837799 [details] [diff] [review]
accessFilter.patch

[Approval Request Comment]
User impact if declined: user with screen reader don't see the checked state
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low
Attachment #8837799 - Flags: approval-comm-beta?
Attachment #8837799 - Flags: approval-comm-aurora?
Attachment #8837799 - Flags: approval-comm-beta?
Attachment #8837799 - Flags: approval-comm-beta+
Attachment #8837799 - Flags: approval-comm-aurora?
Attachment #8837799 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.