Closed Bug 1589859 Opened 10 months ago Closed 5 months ago

Partially broken accessibility in the headers pane for a message

Categories

(Thunderbird :: Message Reader UI, defect, P1)

defect

Tracking

(thunderbird_esr68+ affected, thunderbird75+ affected, thunderbird76 fixed)

RESOLVED FIXED
Thunderbird 76.0
Tracking Status
thunderbird_esr68 + affected
thunderbird75 + affected
thunderbird76 --- fixed

People

(Reporter: k_kolev1985, Assigned: aleca)

References

Details

(Keywords: access, regression)

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

  1. Launch a screen reader like NVDA or Narrator.
  2. Launch Thunderbird 68.
  3. Make sure that the message preview pane is enabled.
  4. Navigate to/select a folder containing a few messages.
  5. Select a message from the list, so it appears in the message preview pane.
  6. Use the TAB key to navigate between the controls in the headers pane (e.g. "From", "Subject", etc.) in the message preview pane.

Actual results:

Controls/fields such as "From", "To", "Date", etc. do not have labels for screen readers - only the content is read (e.g. the name of the sender is read, but not the associated with it label "From").

Expected results:

Controls/fields such as "From", "To", "Date", etc. should have labels for screen readers as they did before version 68 of Thunderbird.

Alex and Jean-Philippe, can you confirm?

Flags: needinfo?(jpmengual)
Flags: needinfo?(aarnaud)

Yes I confirm. I had not seen this because blind people disable the display pane, usually. I disable it and prefer opening the message with Enter. But for those using the display pane, indeed, this should be fixed.

Best regards

Thanks.

Flags: needinfo?(jpmengual)
Flags: needinfo?(aarnaud)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1

But this doesn't affect the standalone windows?
If it does, this is pretty serious - and hard to imagine this isn't a duplicate report.

Component: Disability Access → Message Reader UI
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(jpmengual)
See Also: → 1589862

yes it does. If you:

  1. Press enter on a message to open it in a new window (dont know how happens with open in tabs, I disabled it as buggy)
  2. Press tab and shift tab to reach the message info pane

Result: the same as this described in this bug. You here the sender (but not the label "From"; then, "Subject" (not more), then "Me" and your address, but you dont have label

Likely same as bug 1493608.

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(jpmengual)
Assignee: nobody → alessandro
Status: NEW → ASSIGNED

This patch brings back the proper accessibility labels for all those header fields, and fixes the tests disabled in bug 1493608.

Attachment #9133994 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9133994 [details] [diff] [review]
1589859-accessibility-header.diff

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

::: mail/base/content/mailWidgets.js
@@ +381,1 @@
>  

was role necessary?
textbox is a bit weird for this. See bug 1489748 comment 37.

@@ +2287,5 @@
>        });
>        for (let css of fixedClassed) {
>          pill.emailInput.classList.add(css);
>        }
> +      pill.emailInput.setAttribute("labelID", element.getAttribute("labelID"));

Instead of a custom labelId attribute, could this be using  aria-labelledby?

::: mail/test/browser/message-header/browser_messageHeader.js
@@ +434,5 @@
>    // make sure it loads
>    assert_selected_and_displayed(mc, curMessage);
>  
>    headersToTest.forEach(verify_header_a11y);
> +}); // disabled for now, see bug 1489748

please remove
Attachment #9133994 - Flags: review?(mkmelin+mozilla)

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

was role necessary?
textbox is a bit weird for this. See bug 1489748 comment 37.

The test was looking for that role.
Based on this https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/textbox_role, the role doesn't seem to be related to anything XBL, but rather used to identify non input elements that allow free-form text.
Do we want that?
If it's not necessary I can remove that check from the test.

Instead of a custom labelId attribute, could this be using aria-labelledby?

That was my first approach but apparently aria-labelledby overrides aria-label.
I would have to remove the aria-labelledby attribute once set, that's why I went with a custom attribute.

From 1489748 comment 37 is sounds like we should not set a role for it.

Maybe it's still preferable to set the aria-labelledby and maybe nuke it if then later needed

Updated!

Attachment #9133994 - Attachment is obsolete: true
Attachment #9134193 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9134193 [details] [diff] [review]
1589859-accessibility-header.diff

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

::: mail/test/browser/message-header/browser_messageHeader.js
@@ -392,5 @@
>      `didn't find accessible element for header '${aHeaderInfo.headerName}'`
>    );
>  
> -  Assert.equal(
> -    headerAccessible.role,

should probably remove the expectedRole data for the tests too
Attachment #9134193 - Flags: review?(mkmelin+mozilla) → review+

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

Comment on attachment 9134193 [details] [diff] [review]
should probably remove the expectedRole data for the tests too

I thought I removed everything, is there any leftover?

I was confused.

Duplicate of this bug: 1493608

Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/d51e4a31fe1d
Fix accessibility labels in message pane headerarea. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=d51e4a31fe1d03d1ce4c068b522aed0376174e7a

This caused bct3 test failure which didn't appear in the try-run.
Creating a patch right now.

Attached patch 1589859-bustage-fix.diff (obsolete) — Splinter Review

For some strange reason, the mail-newsgroup element doesn't seem to get the aria-labelledby element.
This fixes it and those tests pass locally.
Here's an artifact try-run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ff08ca70e2786377cce0815dbfb3754eeb2cb94f

Attachment #9134801 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9134801 [details] [diff] [review]
1589859-bustage-fix.diff

Empty patch
Attachment #9134801 - Flags: review?(mkmelin+mozilla)

Darn, sorry about that.

Attachment #9134801 - Attachment is obsolete: true
Attachment #9134855 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9134855 [details] [diff] [review]
1589859-bustage-fix.diff

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

This is not correct, it would just be papering over the problem.
Attachment #9134855 - Flags: review?(mkmelin+mozilla) → review-

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/67ab25b59f96
followup: fix mistakenly removed aria-labelledby. rs=bustage-fix

Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED

You were removing the parent attribute inside a loop.

Target Milestone: --- → Thunderbird 76.0

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

You were removing the parent attribute inside a loop.

Ah, I totally missed that.
Thanks for fixing it.

Regressions: 1637536
You need to log in before you can comment on or make changes to this bug.