1552212 - a11y: listitem combo box with weird name

Status: RESOLVED WORKSFORME

(Thunderbird :: Message Compose Window, defect)

Description

[Jean-Philippe MENGUAL]

Attachment: accerciser outpout when the caret is on the problematic widget

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

1. ctrl-n
2. The caret i in the field to enter the destination addresses.
3. The lisitem is called:
   "To: To: Cc: Bcc: Reply-To: Newsgroup: Followup-To: "

Actual results:

As a result: the assistive technology, instead of speaking "To", speaks "To: To: Cc: Bcc: Reply-To: Newsgroup: Followup-To:".

The attached file shows what happens in Accerciser from a11y point of view.

Expected results:

This lisitem should be named "To", as the various names are the possible choices in the parent combo box, not the title of the lisitem. Maybe we just should not label the listitem, or not doing a wrong relationship.

Comment 1

[Jean-Philippe MENGUAL]

Note that it is a regression as 60 does not have such behavior. As well as beta 65 iirc
Regards

Comment 2

[Samuel Thibault]

It indeed seems odd that there is a label-for/labelled-by relation between the combobox and the autocompletion, while the autocompletion already has a proper accessible name

Comment 3

[Jorg K (CEST = GMT+2)]

Magnus, we need to collect all those a11y issues and address them.

Comment 4

[Magnus Melin [:mkmelin]]

I'm not sure what to look for in that screenshot (what's wrong or not).
Is it possible to look at the a11y using the dev tools accessibility inspector.

Code should be around here:
https://searchfox.org/comm-central/rev/a1a23f8407df96699ab997e6bb619a09fe67e5db/mail/components/compose/content/messengercompose.xul#2040

So addressCol1#1 should have some aria attribute? aria-label maybe, and that should get updated to whatever is used?

Comment 5

[Samuel Thibault]

Attachment: thunderbird.png

The bogus part is "Pour: Pour: Copier[...]". I have here attached an english version where Accerciser has the problematic widget selected: the list item text is "To: To: Cc: Bcc: Reply-To: Newsgroup: Followup-To:", i.e. the whole list of selectable items, instead of just the currently selected item, as it was the case before.

The aria-labelled-by relation is already set between addressCol2#1 and addressCol1#1, the problem here is rather that addressCol1#1 provides spurious text as mentioned above.

I can't seem to run the dev tool inspector in the composition window, ctrl-shift-i does not have effect there, while it does have effect in the main window but the obtained inspector from there does not show the composition window.

Comment 6

[Richard Marti (:Paenglab)]

(In reply to Samuel Thibault from comment #5)

I can't seem to run the dev tool inspector in the composition window, ctrl-shift-i does not have effect there, while it does have effect in the main window but the obtained inspector from there does not show the composition window.

Open the inspector from the main window. When the composer is open, click in inspector on top right on the icon that looks like a window. In the popup select the entry that ends with "messengercompose.xul". Et voilà, you can inspect the composer window.

Comment 7

[Samuel Thibault]

Attachment: thunderbird.png

Ok, here is what it looks like in that inspector, we see the spurious "To: To: Bcc: ..." instead of just "To:"

Comment 8

[Samuel Thibault]

I bisected between nightly builds from https://archive.mozilla.org/pub/thunderbird/nightly/2019/05/, the regression happened between 2019-05-02-07-42-38-comm-central/ and 2019-05-03-07-10-49-comm-central/ : in the former the "listbox list option" text is only "To:", and in the latter it additionally has all choices.

Comment 9

[Jorg K (CEST = GMT+2)]

Samuel, if you bisect, please provide the build changeset which you find in the Troubleshooting Information under about:buildconfig. That saves us to download and check for yourselves:
2nd May: 553df5c9ee2143021b180495409f6c9d98973822
3rd May: 17a0eac83bc55c87a6f0dbe706c78b06d99a8354
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=553df5c9ee2143021b180495409f6c9d98973822&tochange=17a0eac83bc55c87a6f0dbe706c78b06d99a8354

I don't see a C-C change in that range. Looking at M-C, this certainly comes from here:
37d758a90ed9fc26c1d09993ddf73823295ea5d0 Brian Grinstead — Bug 1500626 - Convert <menuitem> bindings to a Custom Element r=surkov
landed 2019-05-03 06:30 CEST, just in time for the 3rd May Daily.

Any hints, Brian?

Comment 10

[Samuel Thibault]

Ah, I didn't know about about:buildconfig.

I tried to revert that commit 37d758a90ed9fc26c1d09993ddf73823295ea5d0, and it indeed does fix the issue for me.

Comment 11

[Brian Grinstead [:bgrins]]

What's the DOM structure here? If this is related to the <menuitem> conversion then I guess there is a <richlistitem> with <menuitem>s beneath it.

A couple notes:

• Menuitems use a special XUL accessibility class (XULMenuitemAccessible) so we explicitly [aria-hidden] on the markup beneath it (which used to be in XBL anon content and ignored): https://searchfox.org/mozilla-central/rev/b3b401254229f0a26f7ee625ef5f09c6c31e3949/toolkit/content/widgets/menu.js#16.
• I wonder if the XULListitemAccessible is somehow treating aria-hidden content beneath it differently than it used to treat xbl. Something interesting is that it seems that class explicitly allows XBL children: https://searchfox.org/mozilla-central/rev/b3b401254229f0a26f7ee625ef5f09c6c31e3949/accessible/xul/XULListboxAccessible.cpp#437-439 so I wonder if there's still XBL involved in the tree here somehow.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: addressing widget.png

Does this answer the question?

The possible labels are To:, Cc:, Bcc: and so on. I guess they are put together on the fly since the user can add their own headers here.

I don't understand why they all got lumped together in the "listbox rich option" in attachment 9071875 [details].

I think Magnus will have to answer all further questions.

Comment 13

[Jorg K (CEST = GMT+2)]

Brian, do you need more info here?

Comment 14

[Brian Grinstead [:bgrins]]

(In reply to Jorg K (GMT+2) from comment #13)

Brian, do you need more info here?

I don't have cycles to debug this right now, but it looks like this is assigned now anyway. If any specific accessibility questions come up you can ask :surkov.

Comment 15

[Alessandro Castellani [:aleca]]

Attachment: richlistbox-label.png

I don't know if I'm following a completely wrong lead, but it looks like that all those extra labels are set due to this getter method: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/richlistbox.js#915

This seems odd as it loops through all the XUL child nodes and returns a joined string of all the child labels.
As you can see from the screenshot, removing that getter lets the richlistitem properly inherit the currently selected menulist label.

Am I completely wrong?

Comment 16

[Jorg K (CEST = GMT+2)]

Thanks for the research. I'm the wrong person to ask. Sounds plausible, no idea why the getter is there and why it strings those labels together. You can always attach a patch with this removed and ask for feedback.

Comment 17

[Jorg K (CEST = GMT+2)]

Alexander, you added the code in question here:
https://hg.mozilla.org/mozilla-central/rev/3a0bd44bd411#l6.97
Can you please take a look at comment #15.

Comment 18

[Brian Grinstead [:bgrins]]

It's possible we are selecting more <label>s from https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/toolkit/content/widgets/richlistbox.js#918-920 now that the inner content isn't anonymous anymore (if we are dealing with an extended richlistitem that used to have XBL <content>.

I'm not sure exactly why that code was originally added - it was moved over in the de-XBL conversion. It's possible that we could drop it, though I'd like to have a pretty thorough accessibility check before we did so.

Comment 19

[alexander :surkov (:asurkov)]

Richlistitem shouldn't look into labels of menulist of course (see comment #17). I suppose prior to de-XBLization those labels weren't visible for richlistitem.label, because they were hidden under XBL anonymous content, now they are in light DOM (explicit content), so richlistitem.label picks them up.

The code was added in bug 281538, 15 years ago, for downloads manager accessibility. It's quite possible the code is obsolete now, and we don't have to concatenate all label elements to come with a label text (accessible name). Having said that richlistitem for sure needs a label (either computed via label property or provided otherwise).

So I'd say that all richlistitem use cases should be examined to find out what makes their labels, and then adjust label property implementation or replace it on something else. For example, If label elements are direct children, then make richlistitem.label to look into direct children only. Or if a first label is only a valueable label, then use it only. Also the author can set aria-label or aria-labelledby on a richlisteim element to define its accessible name. Another a quick fix could be: filtering out labels from menulist elements - not nice but workable.

Comment 20

[Jean-Philippe MENGUAL]

Hi,

I dont know what was done, but in practical, this bug seems fixed. I mean that now, when I tab on the "To" field, I have displayed only "o" and not all the contents of the list. Seems fixed since 3 days about.

Thanks then :)

Regards

Comment 21

[Magnus Melin [:mkmelin]]

Thanks for the update! Maybe fixed by bug 565075 then.