Skip to content

fix(aria-describer): messages not being read out in IE and Edge #12304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

crisbeto
Copy link
Member

Fixes the messages from the AriaDescriber not being read out in Edge or IE, because the message container is aria-hidden and has display: none.

Fixes #12298.

@crisbeto crisbeto added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release labels Jul 21, 2018
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 21, 2018
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jul 24, 2018
// We only disable `aria-hidden` for these platforms, because it comes with the
// disadvantage that people might hit the messages when they've navigated past
// the end of the document using the arrow keys.
messagesContainer.setAttribute('aria-hidden', canBeAriaHidden + '');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I think we do canBeAriaHidden.toString()? Wouldn't that work as well? seems a bit more consistent with the approach we also use for setting the aria-disabled state on other components.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer `${canBeAriaHidden}` but don't really care that much

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just an optional nit.

@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch 2 times, most recently from d4fe09a to f4b4bf5 Compare January 6, 2019 20:27
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from f4b4bf5 to 9964648 Compare February 18, 2019 19:10
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from 9964648 to 498db46 Compare February 28, 2019 21:38
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from 498db46 to a187f26 Compare March 29, 2019 21:22
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from a187f26 to 80192c9 Compare April 9, 2019 20:55
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@andrewseguin andrewseguin added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label May 30, 2019
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from 80192c9 to 9f98064 Compare June 27, 2019 19:09
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from 9f98064 to 46bcd47 Compare August 25, 2019 14:03
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from 46bcd47 to a44322f Compare December 1, 2019 16:39
@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Dec 28, 2019
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from a44322f to abdb461 Compare December 28, 2019 12:47
@crisbeto
Copy link
Member Author

Bumping this to a P2 since this could make some components unusable on IE/Edge with assistive technology. I also spent some time getting the CI to pass again.

Fixes the messages from the `AriaDescriber` not being read out in Edge or IE, because the message container is `aria-hidden` and has `display: none`.

Fixes angular#12298.
@crisbeto crisbeto force-pushed the 12298/aria-describer-ie branch from abdb461 to 7ace81c Compare April 1, 2020 21:08
@andrewseguin andrewseguin merged commit 0e22019 into angular:master Jul 14, 2020
andrewseguin pushed a commit that referenced this pull request Jul 14, 2020
Fixes the messages from the `AriaDescriber` not being read out in Edge or IE, because the message container is `aria-hidden` and has `display: none`.

Fixes #12298.
ngwattcos pushed a commit to ngwattcos/components that referenced this pull request Jul 20, 2020
…lar#12304)

Fixes the messages from the `AriaDescriber` not being read out in Edge or IE, because the message container is `aria-hidden` and has `display: none`.

Fixes angular#12298.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NVDA doesn't announce matTooltip in IE 11
6 participants