todd.kloots Reporter
	Description • 9 years ago

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.125 Safari/537.36

Steps to reproduce:

1. Open the attached example
2. Start the NVDA screen reader
3. Toggle off the screen reader's virtual navigation
4. Press the j or k key to move focus between the items in the list


Actual results:

After you move past the first item the screen reader will announce the label of the previous item along with the label of the next item.


Expected results:

Only the label of the currently focused item should be announced by the screen reader.

	todd.kloots Reporter
	Comment 1 • 9 years ago

Wanted to add this is the root cause of an accessibility bug with twitter.com. It was reported to me last week by one of our visually impaired users.

	Marco Zehe (:MarcoZ)
	Comment 2 • 9 years ago

Thanks, Todd, for filing this!

Alex, can you take a look at this soon'ish? It impacts virtually everyone trying to use Twitter with Firefox.

Allow me to quote from Jamie's analysis in the corresponding e-mail thread:

>It looks like you're changing the label for the *previously* focused item (perhaps removing aria-labelledby?) whenever j or k is pressed. I'm not sure what order you're doing things in, but NVDA sees the following sequence when j or k is pressed:
>1. NameChange for the previously focused item.
>2. NameChange for the item about to be focused.
>3. Focus for the new item.
>The problem is that (1) happens before (3), so NVDA reads the new name because the (1) item is still focused. This means one of two things is happening:
>1. You're changing aria-labelledby on the previous item before focusing the new item.
>2. You're changing aria-labelledby on the previous item *after* focusing the new one, but Firefox is coalescing the events and reordering them. If this is true, imo, this is a Firefox bug.

I can confirm this bug both on Twitter and the attached test case.

	Marco Zehe (:MarcoZ)
	Comment 3 • 9 years ago

(In reply to todd.kloots from comment #0)
> Steps to reproduce:
> 
> 1. Open the attached example
> 2. Start the NVDA screen reader
> 3. Toggle off the screen reader's virtual navigation

For those of you not using NVDA on a daily basis, the keystroke for that is NVDA-Key+Space. This is normally the Insert key, but can be reconfigured to be CAPS Lock, too, in NVDA's Keyboard Preferences.

	alexander :surkov (:asurkov)
	Comment 4 • 9 years ago

So they do
1) blur on the current item (focus goes to document, event will be coalesced later)
2) change name of the current item
3) change name on the next item
4) focus the next item

I tried the testcase DOMi in Nightly
(1) is not focused, (2) is focused. So it looks like there's no problem on Firefox side. Jamie, can you please double check that?

	Trevor Saunders (:tbsaunde)
	Comment 5 • 9 years ago

(In reply to alexander :surkov from comment #4)
> So they do
> 1) blur on the current item (focus goes to document, event will be coalesced
> later)
> 2) change name of the current item
> 3) change name on the next item
> 4) focus the next item
> 
> I tried the testcase DOMi in Nightly
> (1) is not focused, (2) is focused. So it looks like there's no problem on
> Firefox side. Jamie, can you please double check that?

what do you mean "is not focused"? according to comment 2 if the way you say something is focused is it is the target of the previous focus event then the first element is focused.  So if you ask firefox what is focused I believe it will tell you 2 is focused which is right, but if you just use focus events then I suspect you see what Jamie is.

	alexander :surkov (:asurkov)
	Comment 6 • 9 years ago

(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> (In reply to alexander :surkov from comment #4)
> > So they do
> > 1) blur on the current item (focus goes to document, event will be coalesced
> > later)
> > 2) change name of the current item
> > 3) change name on the next item
> > 4) focus the next item
> > 
> > I tried the testcase DOMi in Nightly
> > (1) is not focused, (2) is focused. So it looks like there's no problem on
> > Firefox side. Jamie, can you please double check that?
> 
> what do you mean "is not focused"? 

doesn't have focused state

> according to comment 2 if the way you say
> something is focused is it is the target of the previous focus event then
> the first element is focused.

this is incorrect in general because events delivering happens async. Screen readers should rely on this if they do.

	James Teh [:Jamie] Assignee
	Comment 7 • 9 years ago

Checking for the focused state on every single event would be a nasty perf hit. Also, many frameworks don't set the focused state correctly. In fact, Firefox used to fail to expose focused in some cases. I'm not sure if these bugs still exist, but we certainly still have Mozilla specific code lying around to handle this problem.

Todd, is there any reason you can't:
1. Change the label of the next item.
2. Focus the next item.
3. Only then change the label of the previous item.

	todd.kloots Reporter
	Comment 8 • 9 years ago

Jamie,

Your proposed solution of removing the previous label after focusing the new element works. Thanks for the work-a-around.

Todd

	todd.kloots Reporter
	Comment 9 • 6 years ago

David, Marco:

Any chance we could get some movement on this bug? I'm now working at Slack and our customers using screen readers are suffering this same bug in Slack when using Firefox.

Todd

	James Teh [:Jamie] Assignee
	Comment 10 • 6 years ago

(In reply to alexander :surkov from comment #6)
> > according to comment 2 if the way you say
> > something is focused is it is the target of the previous focus event then
> > the first element is focused.
> this is incorrect in general because events delivering happens async. Screen
> readers should rely on this if they do.

In principle, I disagree here. The fact that event delivery is async isn't relevant here; even if we listened to events synchronously, this would still be wrong, since the nameChange event would still happen before the focus event. The issue here is the ordering of events: clients are finding out about the focus change *after* the name change, which is not actually what happened in reality.

Having said that, I totally get that coalescing events is important. If it's really too hard to make sure the name change event for the previous item lands after the focus event, I could submit a Mozilla specific patch to NVDA to check the focused state for nameChange events. Alex, Marco, thoughts?

(In reply to todd.kloots from comment #9)
> Any chance we could get some movement on this bug? I'm now working at Slack
> and our customers using screen readers are suffering this same bug in Slack
> when using Firefox.

Todd, I assume you're at least blurring the previous element before changing its name? Otherwise, any fix Firefox (or NVDA) could implement would not work.

	todd.kloots Reporter
	Comment 11 • 6 years ago

Jamie,

The implementation in Slack is slightly simpler than what I put in place for twitter.com in that each message has an aria-label value summarizing the content vs using aria-labelledby. But, like twitter.com, the aria-label is applied dynamically in advance of the item being focused. (The label is applied dynamically for performance because it is slightly expensive to compute.) And when you navigate using the up or down arrow keys all I do is move focus between the next or previous item; no blurring the previous element before moving focus to the next as I didn't expect to need to do that. But I can make that change if it fixes the issue and helps customers.

Let me know if you've got any further questions regarding our implementation and/or advice for a fix.

Related question regarding blurring before focusing: imagine you are an average, well-intentioned developer without deeper insight into a11y/and or connections to you all. How would you know the blur is necessary? Most frontend developers wouldn't know IMO. Is it possible to optimize for them and put this fix into Firefox? Asking in earnest.

Todd

	James Teh [:Jamie] Assignee
	Comment 12 • 6 years ago

(In reply to todd.kloots from comment #11)
> The implementation in Slack is slightly simpler than what I put in place for
> twitter.com in that each message has an aria-label value summarizing the
> content vs using aria-labelledby. But, like twitter.com, the aria-label is
> applied dynamically in advance of the item being focused.

Now I'm not sure I understand why this bug strikes at all. If you apply the aria-label to the new item in advance of the focus, why does the previous item get reported? Are you clearing the aria-label on the previous item at the same time?

> But I can make [the blur] change if it fixes the issue and helps customers.

It won't at this stage, but see below for more context.

> Related question regarding blurring before focusing: imagine you are an
> average, well-intentioned developer without deeper insight into a11y/and or
> connections to you all. How would you know the blur is necessary? Most
> frontend developers wouldn't know IMO.

I don't expect developers would know to blur exactly. However, I do expect they would consider the focused item. The issue covered by this bug is that the label for the *previous* item changes before the new item is focused. In the Twitter case, that was a Firefox bug: Twitter blurred item A, changed the label on item A, then focused item B. As far as Twitter was concerned, they changed focus *before* item A's label changed, so it's reasonable to expect that item A's label shouldn't be reported. On the flip side, I don't really understand how we can possibly deal with a case where the author changes the label on item A, *then* focuses item B. As far as a browser or an AT is concerned, the focus was on item A when its label changed, so it's appropriate for the AT to report the label change on item A.

	James Teh [:Jamie] Assignee
	Comment 13 • 6 years ago

To be super clear:
1. If the label of the focused element changes, a screen reader should report the label.
2. So, if you change the label before you move the focus, that label is going to get reported, followed by the new focus.
Is this unreasonable? How can browsers/AT be expected to work around this without introducing arbitrary delays? Is this something you feel most frontend developers wouldn't find intuitive? Honestly not being confrontational; just want to make sure we fully capture the perspectives here.

	todd.kloots Reporter
	Comment 14 • 6 years ago

Ah, I see your point now: the problem is the sequencing of the removal of the aria-label from the previous element. If it happens when that element still has focus, it will be announced is what you're saying. That is where I was getting confused. Sorry for that.

	Pulsebot
	Comment 19 • 2 years ago

Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/337ded6c2bfd
Fire focus events after mutation events but before any other events. r=eeejay

	Pulsebot
	Comment 21 • 2 years ago

Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f8c23f36940
Fire focus events after mutation events but before any other events. r=eeejay