1294038 - [a11y] Autocomplete devtools suggestions selected by keyboard are not read

Status: VERIFIED FIXED

(DevTools :: Shared Components, defect, P3)

Description

[Petruta Horea [:phorea], Desktop QA]

[Note]:
- This issue is happening under Windows with NVDA application.

[Affected versions]:
- 50.0a2 (2016-08-10

[Affected platforms]:
- Win 10 64-bit

[Steps to reproduce]:
1. Open the Developer Tools on a website.
2. On Console tab and on Inspector tab inside "Search HTML" field, start typing an element for which suggestions will be shown (eg "do")
3. Select one of the suggestions by pressing:
 a. Tab key
 b. Up/Down arrow keys followed by Enter 
 c. Up/Down arrow keys followed by Tab

[Expected result]:
- The selected items are read by NVDA.

[Actual result]:
- NVDA doesn't say what the selected item is.

[Regression range]:
- mozilla-central: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=679118259e91f40d4a8f968f03ec4cff066cdb5b&tochange=214884d507ee369c1cf14edb26527c4f9a97bf48
- mozilla-inboud: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=679118259e91f40d4a8f968f03ec4cff066cdb5b&tochange=1de829f2f1f03e23ca0159bee473d36b9989e62b
Regressed by bug 1266456.

[Additional notes]:
- The issue doesn't reproduce when the items are selected using mouse right click.

Comment 1

[Petruta Horea [:phorea], Desktop QA]

*[Affected versions]:
- 50.0a2 (2016-08-10)
- 51.0a1 (2016-08-10)

Comment 2

[Petruta Horea [:phorea], Desktop QA]

Adding updates for Inspector - Rules tab and Style Editor on latest Nightly:

Rules: Property and values suggestion lists are read. When a property is selected using the mouse click, it is read afterwards. When the property is selected using the keys mentioned in comment 0, step 3, it is read as "blank". Values are always read as "blank", no matter how they are selected. 

Style Editor: The suggestions are read while navigating the list using the arrow keys. After they are selected, they are read as "blank".

Comment 3

[Andrew Overholt [:overholt]]

Julian, looks like this regression came from bug 1266456. Can you please take a look?

Comment 4

[Andrew Overholt [:overholt]]

Patrick, do you know if someone can take a look here? Thanks.

Comment 5

[Julian Descottes [:jdescottes]]

I'll take a look at the bug, sorry for not replying earlier.

Comment 6

[Patrick Brosset <:pbro>]

Thanks Julian!

Comment 7

[Julian Descottes [:jdescottes]]

It looks like the root cause of the regression is that since the focus no longer leaves the input to go to the autocomplete popup. 

When the autocomplete popup is closed (when selecting a suggestion for example), the focus doesn't change and the screen reader has no reason to read the value of the input. One of the ways I managed to workaround this is to set the aria-live attribute to "polite" or "assertive" on the autocomplete inputs. I also needed to make sure that the value was updated to trigger the screen reader to read it.

I couldn't find an explicit way to notify that the value of a given input was changed, but I suppose this is by design.

(In reply to Petruta Rasa [QA] [:petruta] from comment #2)
> Adding updates for Inspector - Rules tab and Style Editor on latest Nightly:
> 
> Rules: Property and values suggestion lists are read. When a property is
> selected using the mouse click, it is read afterwards. When the property is
> selected using the keys mentioned in comment 0, step 3, it is read as
> "blank". Values are always read as "blank", no matter how they are selected. 
> 
> Style Editor: The suggestions are read while navigating the list using the
> arrow keys. After they are selected, they are read as "blank".

I thought I already discussed this, but I can't reproduce this consistently. It also happens with FF48 for me, from time to time the screen reader just starts to read "blank" instead of any autocomplete value. I think this is a different issue.

Comment 8

[Julian Descottes [:jdescottes]]

(In reply to Julian Descottes [:jdescottes] from comment #7)
> It looks like the root cause of the regression is that since the focus no
> longer leaves the input to go to the autocomplete popup. 
> 
> When the autocomplete popup is closed (when selecting a suggestion for
> example), the focus doesn't change and the screen reader has no reason to
> read the value of the input. One of the ways I managed to workaround this is
> to set the aria-live attribute to "polite" or "assertive" on the
> autocomplete inputs. I also needed to make sure that the value was updated
> to trigger the screen reader to read it.
> 
> I couldn't find an explicit way to notify that the value of a given input
> was changed, but I suppose this is by design.
> 

Yura: any tip in this kind of situation? The focus remains in the autocomplete's input during the whole flow. Anyway I could trigger the input value to be read by the screen reader at the end?

Comment 9

[Yura Zenevich [:yzen] (please use "needinfo")]

removing ni? for now since we talked in IRC

Comment 10

[Andrew Overholt [:overholt]]

Any chance we can get this fixed before we uplift to beta?

Comment 11

[Julian Descottes [:jdescottes]]

I don't think so, my current attempts are not successful. I need more time to investigate this. 
Marking as wontfix for 50.

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Any progress here?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Pretty crummy that we've shipped this in one release already, and we're in danger of making that two without progress in the very near future.

Comment 14

[Julian Descottes [:jdescottes]]

Will try to get to it this week, sorry about that.

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

Any progress here?

Comment 16

[Julian Descottes [:jdescottes]]

Trying to investigate this bug again, I want to clarify the scope of the regression here.

* "blank" issue:
  As I said earlier this happens for me from time to time regardless of the Firefox version. 
  I actually had it happen only with FF48 during my tests today. I don't know if this is an
  issue related to devtools, to NVDA, but this is not a regression.

Now for the suggestions which are not read when being selected.

* Inspector HTML search
  With FF 50 or newer the selected item is never read when you use the keyboard that's true. 
  But when testing with FF48 it seems that whatever you select, the first item of the list 
  is the one that is read, even if you didn't select it. I don't think this qualifies as an 
  actual regression. It would be better if the selection was read, but the previous behavior
  was also incorrect.

* Ruleview & Style Editor
  The behavior is the same as in FF48, the selection is not read, so not a regression.

* Console
  This is the only regression I can find between FF48 and Nightly, the suggestion used to be
  read when selecting it with the keyboard and it's no longer the case. Also keeping in mind
  that the selected item was read right before.

Based on this, the STRs are:

- open devtools
- open console
- type "d" in the console input
- (here we should read "d" and the selected item together with its position in the list)
- press UP until you reach "document"
- (here everytime a new item is selected, it should be read as well as its position)
- press TAB

ER : "document" is read (actually FF48 reads "edit frame autocomplete multiline document")
AR : the selection is not read

Comment 17

[Julian Descottes [:jdescottes]]

I can't find any way to fix this using regular aria attributes on the autocomplete widget. 
My only successful approach here is to retrigger a focus of the webconsole input when closing the autocomplete popup.

Doing this in a generic way in the autocomplete widget also proves difficult, because other callers of the autocomplete are taking action on the blur/focus of their input. So I propose to restrict this change to the webconsole input:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=df089c5642e284162614f9e524bde8a0f6f1ec75

Comment 18

[Julian Descottes [:jdescottes]]

Attachment: Bug 1294038 - force selections to be read after webconsole completion;

Review commit: https://reviewboard.mozilla.org/r/110068/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/110068/

Comment 19

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

https://reviewboard.mozilla.org/r/110068/#review111074

This looks good to me, and as you say in the bug, you ddin't found a proper way to do it with `aria-*` attribute.
I wonder if we could have a test for that ?

Comment 20

[Julian Descottes [:jdescottes]]

Hmmm, I guess we could check that the input gets blurred and focused when selecting a value. We can't really check that it triggers the expected screen-reader behavior but that's better than nothing. 

I'll probably attach a test patch a bit later.

Comment 21

[Julian Descottes [:jdescottes]]

Sorry didn't mean to ni? Yura here!

Comment 22

[Julian Descottes [:jdescottes]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/110068/diff/1-2/

Comment 23

[Julian Descottes [:jdescottes]]

Nicolas, I just added the test as mentioned in my previous comment! Feel free to re-review.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce7ae714785932d006dd8138ba86d58676cc3a94

Comment 24

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

https://reviewboard.mozilla.org/r/110068/#review112018

A couple of minor comments, still R+ :)

::: devtools/client/webconsole/jsterm.js:1642
(Diff revision 2)
>      this.lastCompletion = { value: null };
>      this.updateCompleteNode("");
>      if (this.autocompletePopup.isOpen) {
> +      // Trigger a blur/focus of the JSTerm input to force screen readers to read the
> +      // value again.
> +      console.log("@@@@@ clearCompletion", this.inputNode);

is this needed ?

::: devtools/client/webconsole/test/browser_webconsole_autocomplete_accessibility.js:47
(Diff revision 2)
> +  EventUtils.synthesizeKey("VK_TAB", {});
> +
> +  info("Wait for the autocomplete popup to be closed");
> +  yield onPopupClosed;
> +
> +  ok(wasBlurred, "jsterm input received a blur event");

since we already tested it in the `blur` listener, I think we can get rid of this one ?

Comment 25

[Julian Descottes [:jdescottes]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/110068/diff/2-3/

Comment 26

[Julian Descottes [:jdescottes]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

https://reviewboard.mozilla.org/r/110068/#review112018

Thanks for the feedback :)

Started a few retriggers of dt8 on OS X 10.10 opt e10s to be sure (a test failed twice on the previous job)

Also started an additional OSX only try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6093ee31b7da284582c540f9c492015ac369723

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c20a71edfab
force selections to be read after webconsole completion;r=nchevobbe

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4c20a71edfab

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta approval on this when you get a chance.

Comment 30

[Julian Descottes [:jdescottes]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

Approval Request Comment
[Feature/Bug causing the regression]: bug 1266456
[User impact if declined]: Users of the devtools console using a accessibility tools will not always get feedback when using the autocomplete of the console input.  
[Is this code covered by automated tests?]: yes, included in this patch
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: 
yes, requires using NVDA & Windows. STRs:
- open devtools
- open console
- type "d" in the console input
- (here we should read "d" and the selected item together with its position in the list)
- press UP until you reach "document"
- (here everytime a new item is selected, it should be read as well as its position)
- press TAB
=> suggestion should be read 
(please note that sometimes NVDA starts reading "blank" instead of any suggestion, which is another bug which existed prior to the regression. if you are hit by this restart ff/devtools)
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: devtools console only, trigger a refocus of an input
[String changes made/needed]: no

Comment 31

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 32

[Brindusa Tot, Desktop QA]

Verified as fixed on latest Nightly 54.0a1, Build ID 20170216030210, against Windows 10x64 with NVDA application.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

Did we want this on Beta as well?

Comment 34

[Gerry Chang [:gchang]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

Fix a devtools issue using an accessibility tools and was verified. Aurora53+.

Comment 35

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/92cff1713ab8

Comment 36

[Julian Descottes [:jdescottes]]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

Sure, let's uplift to beta. I successfully grafted the patch on beta locally and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=979f6b4da689da695410e73add78b31717dd460e

Approval Request Comment
[Feature/Bug causing the regression]: bug 1266456
[User impact if declined]: Users of the devtools console using a accessibility tools will not always get feedback when using the autocomplete of the console input.  
[Is this code covered by automated tests?]: yes, included in this patch
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: 
yes, requires using NVDA & Windows. STRs:
- open devtools
- open console
- type "d" in the console input
- (here we should read "d" and the selected item together with its position in the list)
- press UP until you reach "document"
- (here everytime a new item is selected, it should be read as well as its position)
- press TAB
=> suggestion should be read 
(please note that sometimes NVDA starts reading "blank" instead of any suggestion, which is another bug which existed prior to the regression. if you are hit by this restart ff/devtools)
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: devtools console only, trigger a refocus of an input
[String changes made/needed]: no

Comment 37

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8833968 [details]
Bug 1294038 - force selections to be read after webconsole completion;

devtools a11y fix, beta52+

Should be in 52.0b9

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/27c14cb4dab5

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/27c14cb4dab5

Comment 40

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using 51.0a1, build ID: 20160810030202, on Windows 10 x64.
I can confirm this issue is fixed, I verified Fx 52.0b9 and Fx53.0a2, build ID: 20170221004019, on Windows 10 x64.

Cheers!