nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2.03k stars 625 forks source link

AriaLabel of element should not be re-read after activedescendent listbox is closed #10887

Open isidorn opened 4 years ago

isidorn commented 4 years ago

VS Code issue https://github.com/microsoft/vscode/issues/89098#issuecomment-601599555

Chrome: if HTML element A has focus and has activeDescendent set to element B and element B is a listbox on closing of B arialabel of A should not be re-read.

This works 100% correct in Orca, thus adding @joanmarie for potential ideas / hints. This also does not work on mac VoiceOver.

fyi @zersiax @leonardder

I am also open for potential workarounds on the vscode side. We would really like to fix this.

LeonarddeR commented 4 years ago

@isidorn Would you be able to provide a quick HTML example? It would really help if we can test this in other browsers as well.

isidorn commented 4 years ago

@leonardder not so trivial. The best I could do is run VS Code in browser and verify that this is reproducable with latest Firefox, latest Chrome. Also happens in latest Safari with VoiceOver.

In case you want to run VS Code in browser, here are easy steps.

  1. Build vscode https://github.com/microsoft/vscode/wiki/How-to-Contribute
  2. yarn web from vscode repositry - will open a browser and run "partial vscode" in it. There you can open a suggest box and verify the issue happens
MarcoZehe commented 4 years ago

The problem I see is that aria-activedescendant, which is used in VS Code, does cause real focus events to fire. This is for those cases where keyboard focus seems to be in two places at once: The editor and the suggestion item that is currently selected. Once aria-activedescendant is cleared, a true focus event gets fired on the item that has DOM focus. In fact, we had a problem in Firefox where we weren't always firing this correctly, causing irritating speech output, or none at all, for NVDA and other AT. I would really be curious to know how Orca suppresses the speaking of the name in this specific case, so am hoping for insight from @joanmarie on this. AFAIK, AT cannot discern a focus event coming from a real DOM focus change or an aria-activedescendant one. Also CC'ing @jcsteh for further input.

isidorn commented 4 years ago

@MarcoZehe thanks a lot for looking into this. We are open for workarounds on the VS Code side. Using aria-owns did not help fixing the issue, more details here

joanmarie commented 4 years ago

@MarcoZehe As best as I can tell, it's a combination of lack of accessible events plus Orca trying to not be chatty. :smile:

You are correct that Orca cannot discern a focus event from a real DOM focus change from one resulting from an aria-activedescendant one.

Regarding the lack of events I mentioned above. Orca does see focus events resulting from the aria-activedescendant changes as the user arrows up and down in the selection list. BUT after arrowing to the desired completion in the suggestions list and pressing Return, I am not seeing any focus/activation events for the editor or the application window. That seems like a bug to me. :woman_shrugging:

Regardless... What I am seeing after the suggestions list is closed are text-changed and caret-moved events. These are emitted as a consequence of the suggested completion being applied inside the editor. This is where Orca's trying to not be chatty kicks in.

When text gets inserted into an entry, and that entry is not the thing Orca thinks has focus, Orca does two things:

  1. Determines if the entry should have focus but wasn't told about it (i.e. via a focus event) because application bugs or whatever. If Orca reaches this conclusion, it proceeds to the next item.
  2. Tries to figure out what caused the insertion and what should be presented (if anything) as a result.

For this particular scenario, the answer to 1) is "yes" and the answer to 2) is "text was autoinserted/autocompleted as a direct consequence of user action". Under these circumstances, it chooses to not re-present the fact that the entry regained focus.

isidorn commented 4 years ago

@joanmarie thanks for the nice explanation.

Just fyi that we are also seeing this issue for our parameter hints widget, if we were to change it to use activedescendent. Currently it uses aria.alert, but we can not really change it due to this. More details here https://github.com/microsoft/vscode/pull/93606

Are there maybe any other creative ideas on what could vscode do differntly to workaround this issue. Or are there any plans to fix this issue on the NVDA side?

joanmarie commented 4 years ago

@isidorn Right now, all I've got is a longshot guess. But I'll toss it out there. Could you set focus back to the entry before the destruction of the popup?

This longshot idea is based on a combination of things I'm seeing in Orca and observations @MarcoZehe made.

Like Marco stated: Screen readers (including Orca) do indeed "have a habit of rereading a hierarchical chain of elements when focus changes." What Orca does, and what I assume other screen readers do, is look for the common ancestor of the old focus and the new focus. The screen reader only wants to present what's changed (i.e. the new ancestry up to, but not including, the common ancestor). Make sense?

With this background in mind, what I noticed for Linux is that the ordering of events is:

In other words, old focus is the accepted suggestion. Its parent is the suggestions list which has just been destroyed. Under these conditions, an AT might not be able to determine the common ancestor between the old focus and the new focus because you cannot (reliably) ascend the accessibility tree of destroyed objects. Thus safest thing under such conditions is to present the entire ancestry of the new focus. :woman_shrugging:

If this is indeed what is going on (disclaimer: might not be; see "longshot" above), then perhaps if you set the focus to the entry prior to destroying the suggestions list, the ancestry between the old focus and the new focus can be determined and there will be less to present to the user. Which brings me to the following:

Lastly, it might be worthwhile to try pairing Marco's suggestion (about setting aria-owns on the editor entry pointing to the suggestions list) with mine with the aim of minimizing the distance to the common ancestor. If the common ancestor between the suggestions list and the editor entry is, say, the top-level window, you'll still have a lot of chattiness.

Hope this makes sense!

isidorn commented 4 years ago

@joanmarie thanks a lot for your suggestion. However I tried setting back focus to the entry while the popup is still visible (and has aria-owns on it) and it did not fix the issue. As soon as the focus is back at the entry the ariaLabel gets read. Note that I "move focus" by setting and removing aria-activedescendant.

isidorn commented 3 years ago

@feerrenrut @leonardder are there any updates on this? How can we best help?

michaelDCurran commented 3 years ago

When the listbox closes from pressing enter, we most definitely receive a focus winEvent (EVENT_OBJECT_FOCUS) for the textArea. In the context of previous comments around screen readers announcing or not announcing focus due to common ancestry: from what I can tell, the textArea and suggestions list are cousins of each other, with their common ancestor being a div with a class of monaco-editor. thus, if focus moves from the suggestions list to the textArea, NVDA will (and should) announce the focus on the textArea.

Re aria-owns: if this was used, it would force the listBox to appear as a direct child of the textArea in the accessibility tree, which seems nice and logical. But it would not stop NVDA from announcing focus if focus went back to the textArea when activeDescendant was cleared. This is because we still always announce focus, even on an ancestor. It is only the common ancestors of the new focus we don't announce.

When NVDA announces focus, it always includes the name as one of the properties it speaks. In this case, Chromeium generates the name from the aria-label or labelledby / what ever. NVDA announcing the name here is correct behavior.

From a user efficiency point of view, I do acknowledge that announcing the name (file name) after the suggestion list closes is redundant and a time waster. But generically speaking, NvDA is not doing anything incorrect here.

Perhaps the idea (in the vscode issue) of clearing the label for a while after a suggestion is dismissed is a good idea? But really, I'm not sure this current behavior is absolutely terrible.

isidorn commented 3 years ago

@michaelDCurran thanks for providing more details. I do understand that this behavior makes sense from the NVDA side, however the experience is not that nice since suggest widget closing happens a lot of times and then each time user has to press some key to stop NVDA from repeadaly announcing the title of the editor. This also happens for other widgets like the parameterHints widget.

I think I even tried doing that ugly workaround of clearing the label for a while after a suggestion is dismissed and that might not have worked so well (I am not 100% sure since I did it a while back). Might there be some other nicer workaround we could do on our side?

akash07k commented 3 years ago

@leonardder @feerrenrut @michaelDCurran Guys, is this bug still into consideration or worked upon? Asking because facing this same issue while in editor and it's super frustrating to hear the title again and again when auto complete pops up. Would be good if it can be fixed as it will help VS Code users a lot.

akash07k commented 3 years ago

@leonardder @feerrenrut @isidorn @jcsteh Any update on this issue guys? It's a long time since it has been even commented.

feerrenrut commented 3 years ago

@akash07k there doesn't seem to be an easy way to resolve this, and the severity is not high enough to prioritize over other work. I'm going to mark these comments as off-topic, since the only distract from the core discussion.

isidorn commented 3 years ago

We had more users complaining about this issue and I do think we should fix it. Once a user accepts a suggestion she knows in what file she is, re-reading the file name just does not make sense.

Are there any plans to change the approach of re-reading on the NVDA side? If not do you have some recommendations on what we should do on the VS Code side? What would be the best way to resolve this?

zersiax commented 2 years ago

@michaelDCurran stated: But really, I'm not sure this current behavior is absolutely terrible.

It is, in fact, terrible. This event gets fired when an autocomplete suggestion gets dismissed or accepted, which can happen several times per line when typing. This means you have to deal with , for example, "main.py Multiline Edit insert_line_here" multiple times per line when constructing your lines. If your computer isn't the fastest, I've seen it add the title of the window to this announcement as well, which is a different bug entirely but compounds this one. It's clear that users are running into this; people have been complaining about this for literally years and it negatively impacts the user experience of VS Code users using NVDA, and not, for example, Orca. While I acknowledge NVDA is probably doing the right thing here, I do think some thought needs to be given if some kind of collaborative effort can make the user experience better for this combination of tools. Currently, the only recourse people have is the VS Code addon here wich has not seen updates in some time. Given the position as one of the more accessible code editors out there, it sounds like a no-brainer to make sure these two open-source projects work together as seamlessly as they possibly can, and this issue currently hampers that to a somewhat significant degree, as the productivity hit while minor, does add up.

LeonarddeR commented 2 years ago

I could have a look into this and we could of course try to make this work better in the VS Code appModule, but still, it would be helpful if there was a small html/javascript example that we can use to repro the behavior and possibly write a system test for. May be there's something in the ARIA auto complete best practices that is similar?

LeonarddeR commented 2 years ago

I tested this with NVDA, JAWS and Narrator, and noticed that all the three screen readers announce the edit control as it received focus again, which is what is happening in fact. So this behaviour is normal.

LeonarddeR commented 2 years ago

I shared some additional thoughts in https://github.com/microsoft/vscode/issues/89098#issuecomment-1030633353 and https://github.com/microsoft/vscode/issues/89098#issuecomment-1030673304. IN short, Code isn't following the ARIA best practice since as far as I can see, aria-controls isn't implemented on the editor. Unless that's done, I see no way in NVDA to fix this reliably.

isidorn commented 2 years ago

@leonardder we can add aria-controls or aria-owns if that will help here. Which of the two should we use here? We can probably change this until the end of this week in VS Code insiders so nvda would get unblocked. Does this make sense?

LeonarddeR commented 2 years ago

@isidorn you only need to add aria-controls on the editor pointing to the id of the list that contains the suggestions. The ARIA example I pointed at is a great example of this. If that's done, I guess I can do something at NVDA's side, given people like @feerrenrut and @michaelDCurran are comfortable with the solution I"ll come up with 😉

isidorn commented 2 years ago

@leonardder thanks, I'll let you know once we have something. I think we need a bit more of a general solution that would work for all editor widgets. I have commented in the VS Code issue.

LeonarddeR commented 2 years ago

Ugh, I tried to write a prototype for NVDA based on the ARIA example, but it turns out to be harder than I thought. Aria-controls points to the suggestions list, but as soon as that list disappears, there is no way using IAccessible2 to get a reference to that list any more. To put it into steps:

  1. Editor has focus. IA2 controllerFor relation yields no results
  2. Type something
  3. Suggestions list pops up, aria-activedescendant is set and therefore focus goes to an item in the list
  4. Suggestion is chosen or dismissed
  5. NVDA tries to check the IA2 relation controllerFor on the editor to see whether focus came from a controlled object, but as the list was already dismissed, it yields no results again.

Another approach is checking the controlled by relation on every focus, and if that applies, cache the controlled by until it gets focus again. I assume this can be pretty costly though and therefore I'm a bit reluctant to do this.

akash07k commented 1 year ago

Any update on this issue?

Adriani90 commented 10 months ago

@joanmarie, @isidorn, @akash07k maybe I have a totally non sense proposal, but did you enable cancellable speech when focus event has expired in NVDA's advanced settings? This might fix the issue.

akash07k commented 10 months ago

Hi @Adriani90 Thanks for the suggestion. I think it is worth trying.

@joanmarie, @isidorn, @akash07k maybe I have a totally non sense proposal, but did you enable cancellable speech when focus event has expired in NVDA's advanced settings? This might fix the issue.

jcsteh commented 9 months ago

Ideally, I think the ARIA autocomplete pattern would be redesigned so that it doesn't mess with focus. Note that as far as accessibility APIs are concerned, DOM focus and aria-activedescendant are the same thing: they both cause focus to move. Instead, I think we should be relying on aria-expanded, aria-controls and selection events. This way, ATs could handle autocompletes in specific ways without pulling focus away from the input control. For example, NVDA could use its suggestions opened and suggestions closed sounds, etc. Unfortunately, that is probably not a realistic solution at this point, largely because it would be a backwards compatibility nightmare.

I've thought about this quite a bit over the past few years, trying to come up with a solution in NVDA which would make ARIA autocompletes nicer without backwards incompatible changes to the ARIA spec. It's a tricky problem to solve. Firefox does expose IA2_STATE_ACTIVE for aria-activedescendant but not DOM focus, so it's theoretically possible to differentiate the two in Firefox. However, this isn't implemented in Chrome, it isn't part of the spec and it probably shouldn't be, given that DOM focus and aria-activedescendant are both (by definition) meant to manipulate the accessibility focus.

Given that, to hack around this:

  1. If focus is fired on a list item, ignore it if the current focus is the controller for the item's list.
    • How do we ignore focus? shouldAllowIAccessibleFocusEvent?
  2. If the current focus has the autocomplete state and focus is fired on it again, ignore it.
    • I'm hoping this will already happen due to the duplicate event check in processFocusNVDAEvent, but I'm not certain.
  3. After #14222, we already have a generic event_selection which will report suggestions if they are controlled by the focus.
  4. One problem is that when the list first appears, the selected list item probably won't fire a selection event because it will likely get added to the tree with aria-selected already set. That means the first suggestion won't be read automatically.
    • This will happen even if aria-selected is set after adding the DOM node to the tree because accessibility tree mutations are often asynchronous.
    • The page could get around this by adding a slight delay between adding the DOM node and setting aria-selected, but that is error prone and unintuitive.
    • We could work around this by checking for a selected item when the suggestions appear, though that is itself problematic; se below.
  5. Another problem is that we need a way for NVDA to know when the suggestions list appeared or disappeared so we can play the suggestions opened and closed sounds.

    • There's no relation change event in IA2, so we don't know when the controllerFor relation changes. Even if there were, we don't know which relation changed, though I guess we could cache that if the focus has the autocomplete state.
    • controllerFor seems significant enough that maybe we could add a dedicated IA2 event for it. That would be inconsistent with other things - IA2 events aren't normally that specific - but the practical benefit for users might outweigh that problem.
    • That said, there's no requirement that the page removes aria-controls or removes the list when the suggestions disappear, so a new event might not even help here. Most pages do, but that doesn't mean all pages will, since I don't think the spec requires it.
    • We could use aria-expanded. The page would set aria-expanded="true" when the suggestions appear and set it to false when they disappear. This would require caching the previous expanded state if the focus has the autocomplete state. Unfortunately, aria-expanded isn't allowed on textbox according to the ARIA spec (and thus Chromium). It doesn't really make sense to make the VS code editor a combobox. But maybe we could request a spec change here. Note that Firefox does support aria-expanded on textbox (contrary to the spec), so we could at least try this out in Firefox. Curiously, the spec has this to say:

      When an element has aria-autocomplete set to list or both, authors SHOULD use the aria-expanded state to communicate whether the element that presents the suggestion collection is displayed.

      aria-autocomplete is supported on combobox and textbox, but aria-expanded isn't supported on textbox. So, I think there's a bug in the spec anyway.

balajip881 commented 5 months ago

@jcsteh , Any update on this issue? Please let us try for a solution as i am unable to write code in macbook with voice over. Please suggest any alternatve code editor without this issue