Closed Bug 1519330 Opened 6 years ago Closed 5 years ago

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

Categories

(Calendar :: General, defect)

Product:
Calendar
Component:
General
Version:
Lightning 6.2.4
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: foss, Assigned: khushil324)

References

Details

(Keywords: access, regression)

Attachments

(1 file, 2 obsolete files)

Bug-1519330_calendar-alarm-widget_to_richlistitem-3.patch
(deleted), patch
pmorris
: review+
mkmelin
: feedback+
pmorris
: approval-calendar-beta+
pmorris
: approval-calendar-esr+
Details | Diff | Splinter Review

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

Steps to reproduce:

  1. Open the NVDA screen reader on Windows or Orca screen reader on GNU/Linux
  2. Create an event that we'll happen in one hour
  3. Add a reminder to one day before
  4. Thunderbird will give you a pop-up window
  5. Press tab to go to detail link
  6. 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

Keywords: access, regression
Summary: Unable to read list of events on reminder window with screen reader → No longer to read list of events on reminder window with screen reader

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"

Summary: No longer to read list of events on reminder window with screen reader → No longer able to read list of events on reminder window with screen reader

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

Component: Untriaged → General
Product: Thunderbird → Calendar
Version: 60 → Lightning 6.2.4

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

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

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.

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?

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.

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

Let's get some action here ;-)

Flags: needinfo?(paul)
Flags: needinfo?(alessandro)

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?

Flags: needinfo?(paul)
Flags: needinfo?(alessandro)

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

So how to proceed?

Flags: needinfo?(mkmelin+mozilla)

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.

Assignee: nobody → khushil324
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mkmelin+mozilla)

Sure.

Thanks for moving this along!

Attachment #9094362 - Flags: review?(paul)
Attachment #9094362 - Flags: feedback?(mkmelin+mozilla)
Status: NEW → ASSIGNED
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
Attachment #9094362 - Attachment is obsolete: true
Attachment #9094362 - Flags: review?(paul)
Attachment #9094362 - Flags: feedback?(mkmelin+mozilla)
Attachment #9094390 - Flags: review?(paul)
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.

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

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

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

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.
Attachment #9094390 - Flags: review?(paul)

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

Attachment #9094390 - Attachment is obsolete: true
Attachment #9094806 - Flags: review?(paul)
Attachment #9094806 - Flags: feedback?(mkmelin+mozilla)
Attachment #9094806 - Flags: feedback?(mkmelin+mozilla) → feedback+
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.
Attachment #9094806 - Flags: review?(paul)
Attachment #9094806 - Flags: review+
Attachment #9094806 - Flags: approval-calendar-esr+
Attachment #9094806 - Flags: approval-calendar-beta+
Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
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

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 71

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.

Flags: needinfo?(aarnaud)

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.

Flags: needinfo?(aarnaud)

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

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

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

Flags: needinfo?(khushil324)

Yeah, sure.

Flags: needinfo?(khushil324)
Regressions: 1584806

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

Blocks: 1584806
No longer regressions: 1584806
Target Milestone: 70 → 7.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: