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

Status: RESOLVED FIXED

(Thunderbird :: Disability Access, defect)

Description

[Joanmarie Diggs]

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.

Comment 1

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

Mass un-assigning bugs assigned to Aaron.

Comment 2

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

I'm pretty sure this UI has been re-written. Could someone re-confirm the bug?

Comment 3

[Wayne Mery (:wsmwk)]

(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?

Comment 4

[:aceman]

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+.

Comment 5

[Wayne Mery (:wsmwk)]

https://live.gnome.org/Accerciser

Comment 6

[:aceman]

So can anybody try it out?

Comment 7

[Joanmarie Diggs]

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. :-/

Comment 8

[:aceman]

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?

Comment 9

[Joanmarie Diggs]

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.

Comment 10

[Koichi Inoue]

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.

Comment 12

[Wayne Mery (:wsmwk)]

from bug: 1333847

I started a build with my patch: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e7893319eb430a5840d0be81e20f34804b88e594

When it finishes the build will be available here: https://archive.mozilla.org/pub/thunderbird/try-builds/[e-mail]-e7893319eb430a5840d0be81e20f34804b88e594/

Comment 13

[Richard Marti (:Paenglab)]

Abdelkrim, the build was successful. Please can you test the build from https://archive.mozilla.org/pub/thunderbird/try-builds/[e-mail]-e7893319eb430a5840d0be81e20f34804b88e594/try-comm-central-win32/ and write here if this approach works?

Comment 14

[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.

Comment 15

[Richard Marti (:Paenglab)]

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.

Comment 16

[aleth [:aleth]]

(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.

Comment 17

[Richard Marti (:Paenglab)]

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".

Comment 18

[alexander :surkov (:asurkov)]

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?

Comment 19

[Abdelkrim BENSAID]

(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.

Comment 20

[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/[e-mail]

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.

Comment 21

[Richard Marti (:Paenglab)]

Abdelkrim, can you test this build: https://archive.mozilla.org/pub/thunderbird/try-builds/[e-mail]-92235a424d15bb8b95d9860d7c81b3903f0c3987/try-comm-central-win32/

Try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=92235a424d15bb8b95d9860d7c81b3903f0c3987

Comment 22

[Abdelkrim BENSAID]

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.

Comment 23

[Abdelkrim BENSAID]

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.

Comment 24

[Richard Marti (:Paenglab)]

(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?

Comment 25

[Abdelkrim BENSAID]

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.

Comment 26

[Abdelkrim BENSAID]

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 "[e-mail]", which makes the checkboxes of the message filters easily accessible:

http://cyber25.free.fr/extensions/new/[e-mail]

Best regards.

Comment 27

[Richard Marti (:Paenglab)]

Now that the builds are working again, the next try: https://archive.mozilla.org/pub/thunderbird/try-builds/[e-mail]-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.

Comment 28

[Abdelkrim BENSAID]

(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.

Comment 29

[Richard Marti (:Paenglab)]

Attachment: accessFilter.patch

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.

Comment 30

[Abdelkrim BENSAID]

(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.

Comment 31

[Richard Marti (:Paenglab)]

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.

Comment 32

[Abdelkrim BENSAID]

(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 33

[:aceman]

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.

Comment 34

[Richard Marti (:Paenglab)]

Attachment: accessFilter.patch

Reordered the lines.

Comment 35

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/931e397b74a79ca2e29628e4d05b683564f2624b

Comment 36

[Richard Marti (:Paenglab)]

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

Comment 37

[Jorg K (CEST = GMT+2)]

Aurora (TB 53):
https://hg.mozilla.org/releases/comm-aurora/rev/b57fa03c1fd476c0a4750ed76c4477989813f9aa

Comment 38

[Jorg K (CEST = GMT+2)]

Beta (TB 52):
https://hg.mozilla.org/releases/comm-beta/rev/c042c72ed340fc7a0e869aa0c4acf84ef663c12e