Bugzilla

Assignee

Updated

•

1 year ago

Severity: -- → S3

Flags: needinfo?(lsmith)

Assignee

Comment 2

•

1 year ago

I forgot to mention, the aria-describedby attribute value is invalid. This attribute needs an ID, not a CSS selector. So right now, I'm pretty sure it's not doing anything?

Blocks: fc-surface

BugBot [:suhaib / :marco/ :calixte]

Updated

•

1 year ago

Accessibility Severity: --- → s2

Keywords: access

Comment 3

•

1 year ago

The severity field for this bug is set to S3. However, the accessibility severity is higher, .
:lsmith, could you consider increasing the severity?

For more information, please visit BugBot documentation.

Flags: needinfo?(lsmith)

Assignee

Updated

•

1 year ago

Severity: S3 → S2

Flags: needinfo?(lsmith)

Comment 4

•

1 year ago

I finally was able to get the callout on my Windows machine with NVDA running and, if it is clicked, NVDA would announce it and all the accessibility properties are rendered good. The issue, IMO, is that the container of the callout has role=alert but no accessible name (which could be aria-labelledby to the ID of the title of the callout) and the content inside the alert has role=alertdialog with proper accessible name - this role requires a focus to be placed in the alertdialog for it to be announced. Otherwise, it's being ignored by the assistive technology.

Even though I had disabled the popup autohide in my browser devtools, but the callout disappeared once I tried to capture it, so no screenshot from me here yet. Still...

Ideally, we'd do the following:

the main callout wrapper to be alertdialog
provide the main alertdialog element with accessible name via aria-label/aria-labelledby to the title and then aria-describedby to the content under the title
remove the inner alertdialog to avoid creating mutliple nested alerts and confusing the assistive technology
move the keyboard focus to the Close button - this would trigger the announcement by a screen reader but also ensures the keyboard-only users can also access the callout (they could always press Escape to dismiss it too)

What do you think, Shane? Maybe these changes could be useful for the component itself so we'd avoid the important callouts to be missed by users of screen readers and keyboard-only users?

Flags: needinfo?(shughes)

Updated

•

1 year ago

Comment 5

•

1 year ago

•

Edited

I think this is possible. It doesn't really have an explicit role at all. It's just a panel in this case, which I guess has an implicit alert role. However, as we'll see below, I'm not sure if we would gain anything in practice from this, because the callout is already focused when it is shown.
This probably isn't very practical, because feature callout is a generic template, and not all feature callouts have titles or content under the title. The content is basically arbitrary. And the feature callout itself can't know in advance what the content is going to be. While it would probably be feasible to set aria-labelledby to the title, if a title exists, there isn't a single element containing all the content under the title. That's a bunch of optional elements that may or may not exist and don't share an ancestor. So it's not really feasible to use ARIA attributes to announce the entire feature callout content.
That should be possible.
This is actually already how Feature Callout works. The first eligible button inside the callout is focused when the callout appears. This auto-focus behavior is working for me, and NVDA reads the focused button immediately.

Maybe what we're supposed to be doing is moving focus to the main callout wrapper first, waiting a moment so NVDA can read it, then moving focus to the dismiss button. Instead, what we're currently doing is moving focus right to the dismiss button. So the callout wrapper is never focused, it just has focus within it. My understanding was that as long as focus is within the callout wrapper, it should be read correctly. But that doesn't seem to be what's happening. Instead, it just reads button Close.

Flags: needinfo?(shughes)

Comment 6

•

4 months ago

Anna, given comment #5, what do you think a fix here should/would look like?

Also, is the status quo definitely S2? We are about to roll this out more widely in US English and I'm trying to work out if this should block rollout.

Flags: needinfo?(ayeddi)

Comment 7

•

4 months ago

(In reply to :Gijs (he/him) from comment #6)

Anna, given comment #5, what do you think a fix here should/would look like?

Also, is the status quo definitely S2? We are about to roll this out more widely in US English and I'm trying to work out if this should block rollout.

I am not aware how to re-test the callout, but according to the test results in the comment #4, the callout is not being announced to a user when it appears, which is an access-S2 issue because it creates unequal experience for assistive technology users.

Also, the panels are not implicitly receiving any role, AFAIK, thus we have to explicitly add role=document and a label.

When there is nothing being announced to a user, I'd say aria-labelledby or aria-describedby reference to the entire role=document element would remove the blocker while still being an access-S3 or S4 issue, yet a user would at least be aware where the focus was moved and what's the purpose of this dialog.

Flags: needinfo?(ayeddi)

Assignee

Comment 8

•

4 months ago

Some issues might have been incidentally resolved by changes to feature callout structure over the last year, and there might be new issues specific to Review Checker callouts. You can test them by following these steps:

Go to about:config and enable nimbus.debug (search for that, doubleclick the row)
In a new tab, open about:studies?optin_slug=review-checker-v11-experiment&optin_branch=treatment-a
Go to amazon.com, navigate to a page for a product
The Review Checker sidebar should appear
Click the "X" button to dismiss the sidebar
A feature callout panel should appear

Nowadays, it's not possible to tab-focus the callout itself; only the interactive elements inside it. When it opens, it tries to focus a "primary" interactive element, and if it can't find one it will focus the dismiss button.

Comment 9

•

4 months ago

(In reply to Shane Hughes [:aminomancer] from comment #8)

Some issues might have been incidentally resolved by changes to feature callout structure over the last year, and there might be new issues specific to Review Checker callouts. You can test them by following these steps:

Go to about:config and enable nimbus.debug (search for that, doubleclick the row)

In a new tab, open about:studies?optin_slug=review-checker-v11-experiment&optin_branch=treatment-a

Go to amazon.com, navigate to a page for a product

The Review Checker sidebar should appear

Click the "X" button to dismiss the sidebar

A feature callout panel should appear

Nowadays, it's not possible to tab-focus the callout itself; only the interactive elements inside it. When it opens, it tries to focus a "primary" interactive element, and if it can't find one it will focus the dismiss button.

This focuses the "Got it" button, and for me, only that gets read by NVDA.

(In reply to Anna Yeddi [:ayeddi] from comment #4)

I finally was able to get the callout on my Windows machine with NVDA running and, if it is clicked, NVDA would announce it and all the accessibility properties are rendered good. The issue, IMO, is that the container of the callout has role=alert but no accessible name (which could be aria-labelledby to the ID of the title of the callout)

I don't see an element with this role but perhaps I'm not looking in the right place; I'm not familiar with the internal structure of the callout.

and the content inside the alert has role=alertdialog with proper accessible name - this role requires a focus to be placed in the alertdialog for it to be announced. Otherwise, it's being ignored by the assistive technology.

As far as I can tell this is happening (ie focus is placed there) but I still don't see it being read. Sorry to be asking but would you mind testing these steps again and clarify where the markup is deficient? I'll attach a screenshot.

Flags: needinfo?(ayeddi)

Comment 10

•

4 months ago

Attached image Screenshot of markup and panel — Details

The selected thing in the DOM view has role=alertdialog, and the button for "Got it" is somewhere inside there (and so is the dismiss button).

Assignee

Comment 11

•

4 months ago

•

Edited

(In reply to :Gijs (he/him) from comment #9)

This focuses the "Got it" button, and for me, only that gets read by NVDA.

Thanks for testing. Same for me, NVDA.

(In reply to Anna Yeddi [:ayeddi] from comment #4)

I finally was able to get the callout on my Windows machine with NVDA running and, if it is clicked, NVDA would announce it and all the accessibility properties are rendered good. The issue, IMO, is that the container of the callout has role=alert but no accessible name (which could be aria-labelledby to the ID of the title of the callout)

I don't see an element with this role but perhaps I'm not looking in the right place; I'm not familiar with the internal structure of the callout.

Yeah as you noted, the inner content container's role is currently alertdialog, unless otherwise configured.

A couple years ago, we used to set role=alert on the outermost container. This was when we used <div> rather than <panel> for the container, and callouts were exclusively in-content. This was added in bug 1781088, and removed in bug 1790382.

Assignee

Updated

•

4 months ago

Duplicate of this bug: 1899977

Comment 13

•

4 months ago

Attached image Callout with the updated DOM attributes highlighted and the NVDA output visible — Details

Thank you both for the details, instructions, and testing results!

I was able to reproduce the panel on the Win machine with NVDA. Currently, the heading is announced alongside the CTAs but the sub-heading is missing.

When manually adding an id to the sub-heading element and then referencing this ID as aria-describedby on the main container, the subheading was announced for me.

I wouldn't lower the accessibility severity because still the main piece of information (how to activate the Checker) is still missing from the announcement and it is not expected for a user after hearing the alert to go backwards review the document/callout again.

Flags: needinfo?(ayeddi)

Updated

•

4 months ago

Blocks: 1899575

jchen

Comment 14

•

4 months ago

Just adding a note here agreeing with the P3 designation, I don't think this is a launch blocker.

Assignee

Comment 15

•

4 months ago

(In reply to Anna Yeddi [:ayeddi] from comment #13)

When manually adding an id to the sub-heading element and then referencing this ID as aria-describedby on the main container, the subheading was announced for me.

Excellent, thanks for looking into this Anna. I will add the subtitle (when there is one) as the ARIA description.

I may be doing something wrong (I'm not super experienced with NVDA), but it seems like there may still be another issue in addition to the missing subtitle. If you have some time to do another test, let me know what you think of this:

Set browser.newtabpage.activity-stream.asrouter.devtoolsEnabled to true in about:config
Go to about:asrouter by entering it in the address bar

There should be lots of textboxes in the "Messages" section. In the first one, delete all the contents and replace them with this:

{"id":"NEWTAB_POCKET_CSAT_SURVEY","template":"feature_callout","content":{"id":"NEWTAB_POCKET_CSAT_SURVEY","template":"multistage","backdrop":"transparent","transitions":false,"disableHistoryUpdates":true,"tour_pref_name":"browser.newtab.feature-tour","tour_pref_default_value":"{\"screen\":\"NEWTAB_POCKET_CSAT_SURVEY_1\",\"complete\":false}","screens":[{"id":"NEWTAB_POCKET_CSAT_SURVEY_1","force_hide_steps_indicator":true,"anchors":[{"selector":"hbox#browser","hide_arrow":true,"absolute_position":{"bottom":"20px","right":"20px"}}],"content":{"position":"callout","layout":"survey","title":"Help Firefox improve this page","title_logo":{"imageURL":"chrome://branding/content/about-logo.png"},"subtitle":"How satisfied are you with Recommended by Pocket?","primary_button":{"label":"Submit","action":{"type":"MULTI_ACTION","collectSelect":true,"data":{"actions":[{"type":"SET_PREF","data":{"pref":{"name":"browser.newtab.feature-tour","value":"{\"screen\":\"NEWTAB_POCKET_CSAT_SURVEY_2\",\"complete\":false}"}}}]}},"disabled":"hasActiveMultiSelect"},"additional_button":{"label":"Terms of use","style":"link","action":{"data":{"args":"https://www.mozilla.org/about/legal/terms/mozilla/","where":"tabshifted"},"type":"OPEN_URL"}},"dismiss_button":{"action":{"type":"MULTI_ACTION","data":{"actions":[{"type":"BLOCK_MESSAGE","data":{"id":"NEWTAB_POCKET_CSAT_SURVEY"}},{"type":"SET_PREF","data":{"pref":{"name":"browser.newtab.feature-tour"}}}]},"dismiss":true}},"tiles":{"type":"multiselect","style":{"flexDirection":"column","alignItems":"flex-start"},"data":[{"id":"radio-very-dissatisfied","type":"radio","group":"radios","defaultValue":false,"label":{"raw":"Very Dissatisfied"},"icon":{"style":{"width":"14px","height":"14px","marginInline":"4px 0.5em"}},"action":{"type":"SET_PREF","data":{"pref":{"name":"messaging-system-action.pocket-csat","value":"very-dissatisfied"}}}},{"id":"radio-dissatisfied","type":"radio","group":"radios","defaultValue":false,"label":{"raw":"Dissatisfied"},"icon":{"style":{"width":"14px","height":"14px","marginInline":"4px 0.5em"}},"action":{"type":"SET_PREF","data":{"pref":{"name":"messaging-system-action.pocket-csat","value":"dissatisfied"}}}},{"id":"radio-neutral","type":"radio","group":"radios","defaultValue":false,"icon":{"style":{"width":"14px","height":"14px","marginInline":"4px 0.5em"}},"label":{"raw":"Neutral"},"action":{"type":"SET_PREF","data":{"pref":{"name":"messaging-system-action.pocket-csat","value":"neutral"}}}},{"id":"radio-satisfied","type":"radio","group":"radios","defaultValue":false,"icon":{"style":{"width":"14px","height":"14px","marginInline":"4px 0.5em"}},"label":{"raw":"Satisfied"},"action":{"type":"SET_PREF","data":{"pref":{"name":"messaging-system-action.pocket-csat","value":"Satisfied"}}}},{"id":"radio-very-satisfied","type":"radio","group":"radios","defaultValue":false,"icon":{"style":{"width":"14px","height":"14px","marginInline":"4px 0.5em"}},"label":{"raw":"Very Satisfied"},"action":{"type":"SET_PREF","data":{"pref":{"name":"messaging-system-action.pocket-csat","value":"very-satisfied"}}}},{"id":"radio-dont-use","type":"radio","group":"radios","defaultValue":false,"icon":{"style":{"width":"14px","height":"14px","marginInline":"4px 0.5em"}},"label":{"raw":"I don't use it"},"action":{"type":"SET_PREF","data":{"pref":{"name":"messaging-system-action.pocket-csat","value":"dont-use"}}}}]}}},{"id":"NEWTAB_POCKET_CSAT_SURVEY_2","force_hide_steps_indicator":true,"anchors":[{"selector":"hbox#browser","hide_arrow":true,"absolute_position":{"bottom":"20px","right":"20px"}}],"content":{"layout":"inline","position":"callout","title":"Thanks for your feedback!","title_logo":{"imageURL":"https://firefox-settings-attachments.cdn.mozilla.net/main-workspace/ms-images/706c7a85-cf23-442e-8a92-7ebc7f537375.svg"},"dismiss_button":{"action":{"type":"SET_PREF","data":{"pref":{"name":"browser.newtab.feature-tour"}},"dismiss":true}},"page_event_listeners":[{"params":{"type":"timeout","options":{"once":true,"interval":5000}},"action":{"dismiss":true}},{"params":{"type":"tourend","options":{"once":true}},"action":{"type":"BLOCK_MESSAGE","data":{"id":"NEWTAB_POCKET_CSAT_SURVEY"}}}]}}]},"priority":1,"targeting":"(currentDate|date - profileAgeCreated) / 86400000 >= 28 && 'browser.newtabpage.activity-stream.feeds.section.topstories' | preferenceValue == true","trigger":{"id":"newtabFeatureCalloutCheck"}}

Having replaced all the content, click the "Modify" button next to the textbox
A feature callout micro survey should appear

Here's what NVDA announced for me (this is before adding the subtitle for aria-describedby, but it's the missing button & link I'm worried about):

Help Firefox improve this page  dialog
document
document
Very Dissatisfied  radio button  not checked  1 of 6

I think we'd like to see it say Submit button and Terms of use link at the end. I think the reason the "Got it" button was announced for you in the previous case was simply that the button was focused by default. But with micro-surveys, the radio buttons are focused instead, and the submit button is disabled until a radio button is selected. So neither the button nor the link gets announced, as neither is focused. I'm sure this is a less critical issue, but I'd still like to fix it if you can think of a way.

Thanks again!

Flags: needinfo?(ayeddi)

Updated

•

4 months ago

No longer blocks: 1899575

Comment 16

•

4 months ago

Per discussion in the triage meeting, we believe this doesn't block further rollout in the US.

Comment 17

•

4 months ago

Attached image Manually added ID attribute to the H2 and confirmed NVDA output now announcing the grouping label - the question — Details

(In reply to Shane Hughes [:aminomancer] from comment #15)

I think we'd like to see it say Submit button and Terms of use link at the end. I think the reason the "Got it" button was announced for you in the previous case was simply that the button was focused by default. But with micro-surveys, the radio buttons are focused instead, and the submit button is disabled until a radio button is selected. So neither the button nor the link gets announced, as neither is focused. I'm sure this is a less critical issue, but I'd still like to fix it if you can think of a way.

Thanks again!

Hi Shane, thank you for the STRs - I've got the same results as you did.

I think, if the question would be announced, this would be a sufficient result. Right now, the aria-labelledby attribute from the radiogroup is referring to a non-existent #multi-stage-multi-select-label. It looks like the <h2> is loosing its id attribute during localization/bundling. Thus, the group is rendered as unlabeled and only a radio button's label is announced (and the question from <h2> is not). I manually added that lost id attribute to the <H2> and confirmed NVDA is now announcing the grouping label - the question (screenshot with the DOM and NVDA output attached)

For the footer CTAs, you are right, the "Got it" is announced because it is focused, and with the micro-survey, the radio is being announced - it is expected. In this case, we do not need to worry much about the buttons that are placed after the (focused) radio button in the DOM, because it is expected for a user to continue exploring/reading the page from top to bottom, so these buttons should be found after the radio button. We do not need to announce them. But what's in DOM before the focused element, this is what we need to ensure to announce, because it's not expected for a user to start reading the content backwards to navigate there.

I hope that helps!

Flags: needinfo?(ayeddi)

Jared Hirsch [:jhirsch] (he/him) (Needinfo please)

Assignee

Comment 18

•

4 months ago

(In reply to Anna Yeddi [:ayeddi] from comment #17)

I think, if the question would be announced, this would be a sufficient result. Right now, the aria-labelledby attribute from the radiogroup is referring to a non-existent #multi-stage-multi-select-label. It looks like the <h2> is loosing its id attribute during localization/bundling. Thus, the group is rendered as unlabeled and only a radio button's label is announced (and the question from <h2> is not). I manually added that lost id attribute to the <H2> and confirmed NVDA is now announcing the grouping label - the question (screenshot with the DOM and NVDA output attached)

This #multi-stage-multi-select-label is actually the new way we do questions (sometimes). In normal messages (and in older surveys, like the one in the STR above), we use this generic h2 as the subheading. This is the one you suggested to give id="mainContentSubheader" to, and add aria-describedby to the main content area. So this one wasn't actually losing its id attribute, it just never had an id.

But in the new rollout, we'll be using the #multi-stage-multi-select-label as the subheading. So it shouldn't have that problem, as its h2 will have the correct id="multi-stage-multi-select-label". The new survey's STR are a bit more complicated so I thought I'd spare you the tedium of trying to get it to show up haha.

For the footer CTAs, you are right, the "Got it" is announced because it is focused, and with the micro-survey, the radio is being announced - it is expected. In this case, we do not need to worry much about the buttons that are placed after the (focused) radio button in the DOM, because it is expected for a user to continue exploring/reading the page from top to bottom, so these buttons should be found after the radio button. We do not need to announce them. But what's in DOM before the focused element, this is what we need to ensure to announce, because it's not expected for a user to start reading the content backwards to navigate there.

I hope that helps!

Ah, that does make sense. Thank you, that's great news.

Updated

•

4 months ago

Blocks: 1901619

Jared Hirsch [:jhirsch] (he/him) (Needinfo please)

Updated

•

4 months ago

Assignee: nobody → shughes

Status: NEW → ASSIGNED

BugBot [:suhaib / :marco/ :calixte]

Assignee

Comment 19

•

4 months ago

Attached file Bug 1841374 - Ensure screen readers announce the feature callout subtitle. r=#omc-reviewers — Details

Comment 20

•

3 months ago

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:aminomancer, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Flags: needinfo?(shughes)

Flags: needinfo?(mviar)

Meg Viar [:mviar]

Comment 21

•

3 months ago

•

Edited

We discussed this bug in triage and :aminomancer is planning to land the attached patch.

Flags: needinfo?(mviar)