Skip to content
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

Changes to polite aria-live region are repeated too many times in Chrome and Firefox when title/label of control is changed #7996

Open
alex-barstow opened this issue Feb 13, 2018 · 14 comments

Comments

@alex-barstow
Copy link

alex-barstow commented Feb 13, 2018

Steps to reproduce:

  1. Open this test page in Chrome
  2. Click the button
  3. Listen for repetitive announcements
  4. Repeat 2 and 3 to toggle the button's state

Expected behavior:

The updated text should be announced ~1-2 times (2 would be understandable since the button click updates both thetitle attribute of the button element and the innerText of the child span element)

Actual behavior:

The updated text is repeated 6 times in Chrome (ex. "Text B, Text B, B, Text B, Text B, Text B") and 3 times in Firefox (ex. "Text A, Text A, Text A")

System configuration:

NVDA version: v2017.4

NVDA Installed or portable:
Installed

Windows version:
Windows 10 Pro Version 1709 Build 16299.192

Browser versions:
Chrome 63 and 64
Firefox 58

Other questions:

Does the issue still occur after restarting your PC?
Yes

Have you tried any other versions of NVDA?

Yes, v2015.3 (same results, except there are 5 repetitions instead of 6 in Chrome)

@RichCaloggero
Copy link

RichCaloggero commented Aug 3, 2018

Having same issue

This only occurs with React; does not occur using direct dom manipulation. See more explanation and tests here:

https://github.com/RichCaloggero/react-test

@RichCaloggero
Copy link

THere may be a fix for this

The following github repo contains a set of react components which seem to fix the repetition issue.

So far, only tested in Firefox.

URL: https://github.com/AlmeroSteyn/react-aria-live

@jcsteh
Copy link
Contributor

jcsteh commented Aug 14, 2018

@RichCaloggero: I think the issue you are experiencing is different to this one, even though it has the same symptoms. As discussed via email, yours relates to events being fired for each area of the text that changed, and there wouldn't be "repetition" if aria-atomic wasn't set.

In this test case, the three announcements occur as follows:

  1. The button is focused and its name (as set by the title attribute) changes. Whether or not it's a live region, we would report the change in that case. While I understand this is a distilled test case, there's no good "real world" reason to make a focused button a live region, as its name will always be read when it is changed while it is focused.
  2. The text of the button changes, so a text change event gets fired. NVDA's live region code reports the change.
  3. The name of the button changes (see 1), so a name change event is fired. Because live regions can include graphics, etc. which have names (e.g. alt attribute) that aren't part of the "text", NVDA's live region code also handles name changes. Thus, the change gets reported.

Ideally, we shouldn't do 3) in this case.

@michaelDCurran, my understanding is that we report name changes for live regions because of alt text (#3329). Could we restrict this check so we only do this if the element in question has explicit-name: true? For that matter, should we also avoid retrieving text if explicit-name is true?

I'm not sure why there are even more repeats in Chrome, but this may possibly improve with #8043.

@RichCaloggero
Copy link

Fix with this component

Install react-aria-live from npm. See it's github page here:

https://github.com/AlmeroSteyn/react-aria-live

How it works

Basically, react changes the dom as efficiently as possible. This means, among other things, that if you have some text in a live region and you then update it (i.e. cause the component to update), react will not simply rewrite the entire thing with textContent or appendChild; it will change the actual text nodes. For instance, if you have this:

<div aria-live="polite" aria-atomic="true">
{message}
</div>

and you change message, then text text nodes inside the div will be updated, not removed and rewritten.

The browser will attempt to do this by changing character ranges. For instance if previous contents were "this is a test", and new contents are "this was a simple test", the browser would change three ranges and then fire three events which the accessibility API passes to the screen reader.

The fix then is to empty the container before writing new text to it, but only do this when the new text is different. A bit tricky in react without directly manipulating the dom; use two live regions and alternate between them.

Here is a very simple component that does this, but highly suggest using the react-aria-live component mentioned above as it covers more corner cases and is well written and documented.

class Status extends React.Component {
constructor (props) {
super (props);
this.state = {alternate: false};
} // constructor

static getDerivedStateFromProps(props, state) {
return {alternate: !state.alternate};
} // getDerivedStateFromProps

render () {
  const {alternate} = this.state;
  const {message, ..._props} = this.props;
  
  return (
  <div>
    <div {..._props}>
  {alternate? message : null}
  </div>
      <div {..._props}>
  {!alternate? message : null}
  </div>
  </div>
  );
} // render
}; // Status

@jcsteh
Copy link
Contributor

jcsteh commented Aug 16, 2018

@RichCaloggero commented on Aug 17, 2018, 4:29 AM GMT+10:

Fix with this component

Install react-aria-live from npm. See it's github page here:

@RichCaloggero, as I noted in #7996 (comment), the case you describe is a different case to the one described by the reporter (and thus the subject of this issue). I'd prefer we didn't conflate these two, as while they have the same symptoms, they have very different underlying causes. Thanks.

@jcsteh jcsteh changed the title Changes to polite aria-live region are repeated too many times in Chrome and Firefox Changes to polite aria-live region are repeated too many times in Chrome and Firefox when title/label of control is changed Aug 16, 2018
@Adriani90
Copy link
Collaborator

Small progress in Chrome 74 with NVDA 2019.1.1, the test is repeated only 4 times instead of 6 👍 :-)

chadoh added a commit to AutarkLabs/open-enterprise that referenced this issue Aug 1, 2019
For some reason the `aria-live` elements here seem to get registered
twice, at least by Voice Over. This causes the updated content to be
read twice.

* Click "Delete", Voice Over speaks out "Confirm delete; Confirm delete"
* Click "Edit", Voice Over speaks out the new card contents, then at the
  end says "Cancel; Cancel"
* Click "Cancel", Voice Over speaks out "Confirm Cancel; Confirm Cancel"

I was unable to determine why this happens. The most relevant
information I found is a slightly different problem affecting nvda, a
different screen reader: nvaccess/nvda#7996
chadoh added a commit to AutarkLabs/open-enterprise that referenced this issue Aug 1, 2019
For some reason the `aria-live` elements here seem to get registered
twice, at least by Voice Over. This causes the updated content to be
read twice.

* Click "Delete", Voice Over speaks out "Confirm delete; Confirm delete"
* Click "Edit", Voice Over speaks out the new card contents, then at the
  end says "Cancel; Cancel"
* Click "Cancel", Voice Over speaks out "Confirm Cancel; Confirm Cancel"

I was unable to determine why this happens. The most relevant
information I found is a slightly different problem affecting nvda, a
different screen reader: nvaccess/nvda#7996
Schwartz10 added a commit to AutarkLabs/open-enterprise that referenced this issue Aug 23, 2019
* paste in discussion app

* include discussions in npm start:dev build cmd

* discussion app working

* implement discussions as a useless forwarder

* temp forwarding through discussions

* fix state malfunctions from new aragon-api

* add sample external tx intent

* start integration with new forwarder API

* initial discussion state builder module framework

* build state for discussions in module

* initial integration into dot voting

* log discussion data

* associate discussion thread with dot vote

* display discussion panel and posts

* implement post functionality

* sort discussion posts before serving to react

* small fixes

* disable husky for time being

* programmatically load discussions

* update discussions as they occur

* bug fix

* Discussions: rm some UI that's not in design

* Discussions: extract form to CommentForm

* Clean up styles
* Improve behavior

* Discussions: DiscussionPost → Comment; styling

* Discussions/CommentForm: Add label

* Discussions/CommentForm: add hint text

* Discussions: add UI for edit & reply

These buttons look good, but don't do anything yet

* Discussions: Hook up frontend of comment editing

It needs a working save function in Discussion.js, but the rest is in
place and it behaves properly on the frontend.

* Discussions: allow cancelling comment add/edit

Also, rename `save` prop to `onSave`

* Discussions: confirm cancel

The `40px` bottom margin moved from CommentForm to Discussions because
we don't want the extra space to appear within an under-edit CommentCard

* Discussions: add placeholder Reply behavior

* Discussions/Comment: use actual currentUser

* Discussions/UI: silence propTypes errors

* update package.json with date-fns

* Discussions/Comment: add delete button

Not currently hooked up to API

* remove unused code

* Discussions: remove Reply

We are punting on this feature for now.

Now that Reply has been removed, these are moving to the right, and not
taking up any extra space below the content

* Discussions/Comment: confirm delete

After first click, show "Confirm delete" message. Only delete after
clicking that.

* add hide functionality

* Discussions: address some accessibility concerns

For some reason the `aria-live` elements here seem to get registered
twice, at least by Voice Over. This causes the updated content to be
read twice.

* Click "Delete", Voice Over speaks out "Confirm delete; Confirm delete"
* Click "Edit", Voice Over speaks out the new card contents, then at the
  end says "Cancel; Cancel"
* Click "Cancel", Voice Over speaks out "Confirm Cancel; Confirm Cancel"

I was unable to determine why this happens. The most relevant
information I found is a slightly different problem affecting nvda, a
different screen reader: nvaccess/nvda#7996

* Discussions: render Markdown

* Move the Markdown comment used by Projects' IssueDetail to shared/ui
* Include react-markdown in package.json for Discussions
* use same Markdown component in both places
* fix Hint text to explain how to make things bold in Markdown

* update aragon api versions

* overwrite files that should not have been touched

* remove unused allocations files

* remove untouched project apps files

* one more untouched projects file

* remove untouched rewards files

* correct import to markdown file in projects

* reset dot voting untouched files

* more untouched code removal

* add a useHandshake hook

* fix dot voting lint errors

* use handshake in discussion api

* update packages for new api

* use autark ipfs node

* update radspec

* add discussion documentation

* discussions Readme updates

* remove discussions as a forwarder from dot voting

* Discussions: rm changes to Dot Voting

This is focusing this branch to add the minimal amount of code.

The changes in apps/dot-voting will cause significant conflicts when we
integrate Discussions with the Dot Voting design system upgrade. The
easiest path forward is to remove the changes entirely from this branch,
so they can be added only after the design system upgrade.

* Discussions: rm apps/rewards/package-lock.json

This probably got added by accident; it is not related to Discussions

* Discussions: revert unrelated package.json changes

These changes are unrelated to getting Discussions working; removing
them to make Discussions play more nicely with other branches.

* Discussions: rm obsolete Markdown component

This now lives in shared/ui/components/misc, and should only be used
from that location

* Discussions: rm apps/dot-voting/package-lock.json

This file should not be in this branch and was probably added by
accident.

* Discussions: fix linting errors and &/&& typos

Most of the linting errors were complaining about use of `now`. This [can
cause security problems][1], if using `now` for security-related
functions. We are not doing that here, so I am guessing that our use of
`now` is safe, and am therefore ignoring these lint warnings explicitly.

  [1]: https://consensys.github.io/smart-contract-best-practices/recommendations/#timestamp-dependence

* Fix lint and remove package-lock files

* Update lint script to use correct local config for each app

* hardcode 100 discussion thread Ids

* Discussions: add placeholder UI

This UI shows up when you select Discussions in the sidebar. We have
nothing to show for the Discussions app itself at this point in time,
and now we explain that here.

* Discussions: npm run lint:fix

* update contract
@Yegorich555
Copy link

Yegorich555 commented May 14, 2020

One more issue. If a child contains couple items and you change aria-hidden="true" to aria-hidden="false" it will announce twice! The issue exists with the following html structure:

<div tabindex='0' aria-live='polite>
    <div id='slide-1' aria-hidden="false">
       <img src='...' alt='Banner 1'/>
       <div>
           <p> Some text here</p>
           <p> and here</p>
       </div>
    <div>
   <div id='slide-2' aria-hidden="true">
       <img src='...' alt='Banner 2'/>
       <div>
           <p> Some text here</p>
           <p> and here etc.</p>
       </div>
    <div>
</div>

@Adriani90
Copy link
Collaborator

@Yegorich555 I guess the slide 2 is still visible although aria-hidden=true and this leads to the objects being repeated. Right? Usually aria-hidden=true is used together with display:none or visibility:hidden like in this example:
https://www.w3.org/WAI/PF/aria-1.1/states_and_properties#aria-hidden
Using only aria-hidden=true would suffice for screen readers which access information only through the DOM directly. But as far as I understand NVDA is not using only the DOM but also other accessibility platforms.

But I think @jcsteh could explain much better than me :-)

@Yegorich555
Copy link

Yegorich555 commented May 15, 2020

@Adriani90 you are right. Slide 2 is still visible for a couple seconds, but aria-hidden='true' must fix twice-reading... One more detail: aria-relevant='text' fixes twice-reading but it doesn't work when we have 'div' with nested 'p'. So the following example works for me

<div tabindex='0' aria-live='polite' aria-relevant='text'>
    <div id='slide-1' aria-hidden="false">
       <img src='...' alt='Banner 1'/>
       <div class="visuallyhidden"> Some text here and here</div>
       <div aria-hidden="true">
           <p> Some text here</p>
           <p> and here</p>
       </div>
    <div>
   <div id='slide-2' aria-hidden="true">
       <img src='...' alt='Banner 2'/>
       <div class="visuallyhidden"> Some text here and here etc.</div>
       <div aria-hidden="true">
           <p> Some text here</p>
           <p> and here etc.</p>
       </div>
    <div>
</div>

@AleksandrHovhannisyan
Copy link

This is a bug even without React. Simple demo here: https://gui-challenges.web.app/theme-switch/dist/. The button uses aria-live="polite", but changes are narrated three times in succession.

@RichCaloggero
Copy link

  1. I hear the announcement exactly twice with Firefox and latest NVDA (2021.3.1)
  2. my suspician is that when the label changes, the live region triggers and new text is read, but because the button also has focus it is re-read
    This case should never arrise; why would you put live region marup on a button? If you want to communicate the state of a button, make it a toggle button, or simply change the label as you are doing and NVDA will read the new one (i.e. drop the live region markup and the announcement will only happen once).

@seanbudd

This comment was marked as off-topic.

@AleksandrHovhannisyan

This comment was marked as off-topic.

@seanbudd

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants