1519330 - No longer able to read list of events on reminder window with screen reader

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Alex ARNAUD]

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

Steps to reproduce:

0. Open the NVDA screen reader on Windows or Orca screen reader on GNU/Linux
1. Create an event that we'll happen in one hour
2. Add a reminder to one day before
3. Thunderbird will give you a pop-up window
4. Press tab to go to detail link
5. Press shift+tab to go on the title of the event

Actual results:

The event title is not presented to the user via its screen reader, so in other word: a blind person is no longer able to read the reminder window.

Expected results:

The event title is presented to the user like it was on Thunderbird 52

Comment 1

[Alex ARNAUD]

This is the technical feedback from Joanie posted on Orca mailling list: https://mail.gnome.org/archives/orca-list/2018-November/msg00117.html
(Joanie is the Orca maintainer and has long experience on accessibility on GNU/Linux and the web)

"Hi Alex.

It's clear now. Looking at Accerciser, here are the problems/brokenness:

1. The children of list boxes should be list items; what we have now
   is two labels and a link, and the push button. That makes no sense.
2. When I use the selection interface to see how many selected children
   there are, I'm told one. This is good (insofar as visually, one thing
   is highlighted) however:
3. When I use the selection interface to get the selected child, I get
   no object returned.

Based on the above, I'm wondering if something is causing the list item
to be pruned from the accessibility tree even though it's really there
in the GUI.

This most definitely needs to be fixed in Thunderbird. Sorry.
--joanie"

Comment 2

[Jorg K (CEST = GMT+2)]

Please file Calendar bug in the Calendar product to the Calendar people checking for new bugs in their product can find them.

Comment 3

[Jorg K (CEST = GMT+2)]

(In reply to Alex ARNAUD from comment #1)

1. The children of list boxes should be list items; what we have now
   is two labels and a link, and the push button. That makes no sense.
   Sadly the Mozilla platform removed the "simple" listbox and switched everything to the so-called richlistbox which has a different makeup, as you observed. (Or I maybe on the wrong boat here since I don't know Calendar all that well.)

Comment 4

[Magnus Melin [:mkmelin]]

From the context, I assume he's testing thuderbird 60. <listbox> still exists there, so this is not likely the issue

Comment 5

[Jorg K (CEST = GMT+2)]

Right. For the addressing widget we switched from listbox to richlistbox in TB 60 to fix some other issues, bug that's not related to reminders.

Comment 6

[Peter Vágner]

If you like I can try to find out when this started to be a problem for us however I'd appreciate a hint here.
How could I test lightning builds?
Can you identify corresponding lightning version integrated into a thunderbird daily build or do I need a different strategy for example installing lightning builds into one build of thunderbird?
Might such a thing be also useful here?

Comment 7

[Jorg K (CEST = GMT+2)]

Lightning is shipped with all versions of TB now. Particularly in Daily, you should always get the shipped/packaged version. So if you identify the Daily regression, that would be great. Or we can ask Alice.

Comment 8

[Alex ARNAUD]

(In reply to Jorg K (GMT+1) from comment #7)

Lightning is shipped with all versions of TB now. Particularly in Daily, you
should always get the shipped/packaged version. So if you identify the Daily
regression, that would be great. Or we can ask Alice.

Where is it indicated? Looking at the download page of Thunderbird, I see no reference about the word "lightning" or "calendar" here: https://www.thunderbird.net/en-US/channel/

It's the same on the home page: https://www.thunderbird.net/en-US/

Best regards,
Alex.

Comment 9

[Jorg K (CEST = GMT+2)]

That happened in TB 38: https://www.thunderbird.net/en-US/thunderbird/38.0.1/releasenotes/ ;-)

Comment 10

[Jorg K (CEST = GMT+2)]

Let's get some action here ;-)

Comment 11

[Magnus Melin [:mkmelin]]

This area was changed a lot during de-xbl. It may well have been "fixed".
aarnaud, do you still see this in thunderbird 68 and or on trunk?

Comment 12

[Alex ARNAUD]

(In reply to Magnus Melin [:mkmelin] from comment #11)

This area was changed a lot during de-xbl. It may well have been "fixed".
aarnaud, do you still see this in thunderbird 68 and or on trunk?

Unfortunately, It has not been fixed and I've just checked again, there is another regression here: it is no longer possible to move from one event to another with arrow keys in the reminder window.

Best regards.

Comment 13

[Jorg K (CEST = GMT+2)]

So how to proceed?

Comment 14

[Magnus Melin [:mkmelin]]

Khushil, can you take a look?
I suspect what is wrong is that calendar-alarm-widget is not a richlistitem. If we change it to a customized built-in <richlistitem is="calendar-alarm-widget-richlistitem"> the key navigation is likely to sort it self out, and supposedly the a11y issue too.

Comment 15

[Khushil Mistry [:khushil324]]

Sure.

Comment 16

[Jorg K (CEST = GMT+2)]

Thanks for moving this along!

Comment 17

[Khushil Mistry [:khushil324]]

Attachment: Bug-1519330_calendar-alarm-widget_to_richlistitem.patch

Comment 18

[Magnus Melin [:mkmelin]]

Comment on attachment 9094362 [details] [diff] [review]
Bug-1519330_calendar-alarm-widget_to_richlistitem.patch

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

::: calendar/base/content/widgets/calendar-alarm-widget.js
@@ +214,4 @@
>      }
>    }
>  
> +  customElements.define("calendar-alarm-widget", MozCalendarAlarmWidget, {

please add a trailing -richlistitem to the name, and also update the name accordingly. This makes it more clear what it is.
So calendar-alarm-widget-richlistitem

Comment 19

[Khushil Mistry [:khushil324]]

Attachment: Bug-1519330_calendar-alarm-widget_to_richlistitem.patch

Comment 20

[Magnus Melin [:mkmelin]]

Comment on attachment 9094390 [details] [diff] [review]
Bug-1519330_calendar-alarm-widget_to_richlistitem.patch

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

Should also hg mv the file to reflect the new class name.
Anyway, this looks good, and keyboard navigation works. 

The dialog it self doesn't work very well though (not because of this patch). Encountered a  few related. Filed bug 1583098 and bug 1583101.

Comment 21

[Khushil Mistry [:khushil324]]

(In reply to Magnus Melin [:mkmelin] from comment #20)

Should also hg mv the file to reflect the new class name.
Anyway, this looks good, and keyboard navigation works.

Do you want this CE in a new file?

Comment 22

[Magnus Melin [:mkmelin]]

Ah, didn't notice it had other things there too.
I think not a big deal, but ideally would be one widget per file.

Comment 23

[Magnus Melin [:mkmelin]]

Might cause JavaScript error: chrome://calendar/content/widgets/calendar-alarm-widget.js, line 323: TypeError: eventTarget is null
(see bug bug 1583101)

Comment 24

[Paul Morris [:pmorris]]

Comment on attachment 9094390 [details] [diff] [review]
Bug-1519330_calendar-alarm-widget_to_richlistitem.patch

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

I see some instances that missed the change of adding "-richlistitem".

::: calendar/base/content/widgets/calendar-alarm-widget.js
@@ +62,4 @@
>        );
>        this.mItem = null;
>        this.mAlarm = null;
> +      this.setAttribute("is", "calendar-alarm-widget");

This needs the "-richlistitem" on the end, right?

::: calendar/base/themes/common/dialogs/calendar-alarm-dialog.css
@@ +13,4 @@
>  }
>  
>  /* Alarm widget specific styles */
> +richlistitem[is="calendar-alarm-widget"] {

This needs the "-richlistitem" on the end.  Here and elsewhere in this file.

Comment 25

[Jorg K (CEST = GMT+2)]

When you get the final version, please stick the approval-flags on at when doing the review.

Comment 26

[Khushil Mistry [:khushil324]]

Attachment: Bug-1519330_calendar-alarm-widget_to_richlistitem-3.patch

Comment 27

[Paul Morris [:pmorris]]

Comment on attachment 9094806 [details] [diff] [review]
Bug-1519330_calendar-alarm-widget_to_richlistitem-3.patch

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

LGTM.  With this patch I was able to use up/down arrows to move through the list of reminders.  I don't have a screen reader set up, so haven't tested that.  Per mkmelin, I assume that will work now that the element is a "richlistitem".  I didn't see the error in the console from comment 23.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/052f6dc24699
converted calendar-alarm-widget to <richlistitem is="calendar-alarm-widget-richlistitem"> to solve a11y issues. r=pmorris

Comment 29

[Jorg K (CEST = GMT+2)]

Alex, can you please check the fix in tomorrow's Daily and then we'll get it into TB 70 beta 3 and TB 68.2.

Comment 30

[Alex ARNAUD]

In short: it's better but not fixed because TB doesn't give relevant information but generic ones.

1. We're able to move between events with up/down, great and many thanks.

2. We're hearing general details for each events:
   "details minute minute hour day" (translated from french)
   I assume so there are variables not populated or propagated properly.

Best regards.

Comment 31

[Jorg K (CEST = GMT+2)]

So what exactly do you need to hear? Specific dates and times?

Comment 32

[Jean-Philippe MENGUAL]

Hi,

the initial behavior was displaying and speaking for each line:

• Title of the event
• date and time of starting
• date and time of end

Regards

Comment 33

[Jorg K (CEST = GMT+2)]

Khushil, could you file another bug and look in to that please.

Comment 34

[Khushil Mistry [:khushil324]]

Yeah, sure.

Comment 35

[Jorg K (CEST = GMT+2)]

TB 70 beta 3:
https://hg.mozilla.org/releases/comm-beta/rev/d966389e4958e11153bb3854aebdf4ea43441066

Comment 36

[Jorg K (CEST = GMT+2)]

TB 68.1.2 or TB 68.2 / Cal 7.0:
https://hg.mozilla.org/releases/comm-esr68/rev/f639597f7fb13b5470620a5f810c4b7c4505c0b5