Closed Bug 1590216 Opened 5 years ago Closed 4 years ago

Screen readers fail to announce autofill in address bar when it is focused for the first time

Categories

(Core :: Disability Access APIs, defect, P2)

71 Branch
Desktop
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: wulfryk1, Assigned: morgan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Before reproducing ensure that you haven't disabled autocomplete.

STR:

  1. Start NVDA or JAWS.
  2. Launch Firefox Nightly
  3. Without moving focus from address bar start typing address from history.
  4. Move away from address bar, navigate back to it and repeat step 3.

Expected:
Suggestions should be announced in both cases.
Actual:
Suggestions are announced only after focus is move from address bar.

Thanks for the report, Lukasz. To clarify:

  1. Are you referring to autocomplete suggestions or autofill? That is, are you referring to the list items that are reported if you press down arrow after typing in the address bar (autocomplete suggestions), or are you referring to the text that gets filled into the address bar and selected as you type without you having to press down arrow (autofill)?
  2. If you're referring to autofill, what does NVDA report if you use the report selection command (desktop: NVDA+shift+upArrow, laptop: NVDA+shift+s) after typing?
Flags: needinfo?(wulfryk1)

I am referring to the autofill, - sorry for the confusion.
When pressing command to announce current selection NVDA properly reports the part of the address that is autofilled in both cases the only problem is the fact that autofill is not reported automatically when address bar gains focus for the first time.

Flags: needinfo?(wulfryk1)

Confirmed. I'm trying to work out whether this is a Firefox bug or an NVDA/JAWS bug. Given that it happens in both screen readers, the obvious conclusion is that it's a Firefox bug. However, curiously, this works as expected with Narrator, where I'd least expect it to work. So... that's confusing.

Things to note when looking into this:

  1. You need to have Options -> Restore previous session disabled in order for the address bar to get focus on startup. This is the default, but I had this enabled in my profile and it took me a while to figure this out, so thought it worth noting.
  2. Perhaps we aren't firing caret events for some reason when the address bar gets focused at startup? That would prevent NVDA's auto select detection from working.
Status: UNCONFIRMED → NEW
Component: Disability Access → Disability Access APIs
Ever confirmed: true
Priority: -- → P2
Product: Firefox → Core

If I set A11YLOG to events, I see this for autofill where the address bar is focused at startup:

Selection changed, selection type: normal, notification ignored, reason: 0

If it's focused later, I see "notification pending" instead.

If I log events as well, I don't see a selection or caret event.

logging::SelChange logs "notification ignored" if !aDocument || !aDocument->IsContentLoaded(). I don't think aDocument should be null, but that needs to be checked. Interestingly, even though we check IsContentLoaded, I don't see where in the subsequent code path we actually return early for this. Maybe the document's mNotificationController is null? But I don't think so, because we do fire other events (reorder, etc.) and we couldn't fire those if mNotificationController wasn't created yet.

Morgan, want to take a crack at debugging this? The starting point for all this is here:
https://searchfox.org/mozilla-central/rev/8a63fc190b39ed6951abb4aef4a56487a43962bc/accessible/base/SelectionManager.cpp#141

Assignee: nobody → mreschenberg
Blocks: eventa11y
Summary: Screen readers fail to announce autocomplete in address bar when it is focused for the first time → Screen readers fail to announce autofill in address bar when it is focused for the first time

Interesting: I can reproduce with NVDA on Windows, but starting a fresh (non-restored) copy of nighlty or nigthly debug on MacOS with VoiceOver results in the tab bar getting focus instead of the address bar, from what I can tell.

If I do event logging, I get these two events for both the first focus and last focus:

type: focus
target: [hex address]
role: entry
name: "search with google or enter address"
[... info about android, aria control, aria auto complete, etc.]

type: text caret moved
target: [same above hex address]
role: entry
name: "search with google or enter address"
[... info about android, aria control, aria auto complete, etc.]

@Jamie: I'm not seeing the notification information; is that added with a different flag?
Also: NVDA keeps quitting on me? (no crash notification, but my keyboard commands stop working and it disappears from the windows taskbar) this happens pretty consistently when I do console debugging and close the open windows + restart with ./mach run --console

Flags: needinfo?(jteh)

(In reply to Morgan Reschenberg [:morgan] from comment #6)

If I do event logging, I get these two events for both the first focus and last focus:
...
type: text caret moved

Oh fascinating. So you're seeing a caret moved event. Maybe I'm wrong about this, then.

@Jamie: I'm not seeing the notification information; is that added with a different flag?

Sorry, my bad. I said set A11YLOG to events. I meant selection. But you probably want both: A11YLOG should be selection,events .

Also: NVDA keeps quitting on me? (no crash notification, but my keyboard commands stop working and it disappears from the windows taskbar) this happens pretty consistently when I do console debugging and close the open windows + restart with ./mach run --console

Are you closing the console with the mouse? That's a known issue in NVDA caused by Windows silliness that's difficult to work around. If you close with alt+f4, that shouldn't happen.

Flags: needinfo?(jteh)

(In reply to James Teh [:Jamie] from comment #4)

If I set A11YLOG to events, I see this for autofill where the address bar is focused at startup:

Selection changed, selection type: normal, notification ignored, reason: 0

If it's focused later, I see "notification pending" instead.

If I log events as well, I don't see a selection or caret event.

logging::SelChange logs "notification ignored" if !aDocument || !aDocument->IsContentLoaded(). I don't think aDocument should be null, but that needs to be checked. Interestingly, even though we check IsContentLoaded, I don't see where in the subsequent code path we actually return early for this. Maybe the document's mNotificationController is null? But I don't think so, because we do fire other events (reorder, etc.) and we couldn't fire those if mNotificationController wasn't created yet.

Morgan, want to take a crack at debugging this? The starting point for all this is here:
https://searchfox.org/mozilla-central/rev/8a63fc190b39ed6951abb4aef4a56487a43962bc/accessible/base/SelectionManager.cpp#141

I tried running ./mach mochitest path/to/test --debugger=lldb and setting a breakpoint at SelectionManager::NotifySelectionChanged, but it never breaks there :( were you seeing this locally, or was this a hunch? Trying to make sure my test actually tests the right thing.

Flags: needinfo?(jteh)

(In reply to Morgan Reschenberg [:morgan] from comment #9)

I tried running ./mach mochitest path/to/test --debugger=lldb and setting a breakpoint at SelectionManager::NotifySelectionChanged, but it never breaks there :( were you seeing this locally, or was this a hunch? Trying to make sure my test actually tests the right thing.

Hmm. I was definitely seeing this log message locally:

Selection changed, selection type: normal, notification ignored, reason: 0

I didn't explicitly verify that SelectionManager::NotifySelectionChanged gets called, but I only saw one place which could log that message (logging::SelChange), and SelectionManager::NotifySelectionChanged is the only place I see which calls logging::SelChange.

Of course, it's possible that was actually being hit somewhere else and I got the log confused. But I don't think so.

Flags: needinfo?(jteh)
Blocks: 1604096

(In reply to James Teh [:Jamie] from comment #10)

Of course, it's possible that was actually being hit somewhere else and I got the log confused. But I don't think so.

I took another look at this. I now believe this was a red herring; sorry. I did see that message, but I guess it must have been for some other selection change. When I open a new window and repro the bug that way, I don't see this message.

Another thing I just spotted that I'm suspicious of: we register for selection notifications for the focused element when DOM focus changes:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/accessible/base/FocusManager.cpp#121
and clear it when we get a DOM blur:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/accessible/base/FocusManager.cpp#146
Could something here be failing; e.g. we don't call SelectionManager::SetControlSelectionListener or something in that method fails?

(In reply to James Teh [:Jamie] from comment #11)

Another thing I just spotted that I'm suspicious of: we register for selection notifications for the focused element when DOM focus changes:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/accessible/base/FocusManager.cpp#121
and clear it when we get a DOM blur:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/accessible/base/FocusManager.cpp#146
Could something here be failing; e.g. we don't call SelectionManager::SetControlSelectionListener or something in that method fails?

That sounds indeed interesting. The behavior I observe in bug 1604096 suggests that something fails when a newly created element gets focus right away. It corrects itself once I set focus to something else, then come back. It is not failing with simple testcases that merely insert a textarea, or un-hide it via CSS. So something more complex must be happening to trigger this. But that bug is very similar to this one, IMO. So I believe this is worth investigating further.

(In reply to Marco Zehe (:MarcoZ) from comment #12)

(In reply to James Teh [:Jamie] from comment #11)

Another thing I just spotted that I'm suspicious of: we register for selection notifications for the focused element when DOM focus changes:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/accessible/base/FocusManager.cpp#121
and clear it when we get a DOM blur:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/accessible/base/FocusManager.cpp#146
Could something here be failing; e.g. we don't call SelectionManager::SetControlSelectionListener or something in that method fails?

That sounds indeed interesting. The behavior I observe in bug 1604096 suggests that something fails when a newly created element gets focus right away. It corrects itself once I set focus to something else, then come back. It is not failing with simple testcases that merely insert a textarea, or un-hide it via CSS. So something more complex must be happening to trigger this. But that bug is very similar to this one, IMO. So I believe this is worth investigating further.

ah thanks to both of you! I'll check this out tomorrow :)

Looks like when we call FocusManager::NotifyOfDOMFocus, we're passing in a document instead of the focused object (url bar); at least I think this is true because targetNode->AsDocument() is the same as targetNode->OwnerDoc(). When I call to see if there exists a child of this node which is an element, I get that an HTMLSharedElement exists. Not sure if this could be our url bar, but will investigate further.

(In reply to Morgan Reschenberg [:morgan] from comment #14)

Looks like when we call FocusManager::NotifyOfDOMFocus, we're passing in a document instead of the focused object (url bar); at least I think this is true because targetNode->AsDocument() is the same as targetNode->OwnerDoc(). When I call to see if there exists a child of this node which is an element, I get that an HTMLSharedElement exists. Not sure if this could be our url bar, but will investigate further.

Ah wait scratch this, I found an HTMLInputElement which gets passed into NotifyOfDOMFocus, and SetControlSelectionListener is set :( hmm.

Debugged this a little with :asurkov today (thank you!) and found we're not calling SelectionManager::NotifySelectionChanged even though we are calling EditorBase::NotifySelectionChanged meaning after we set our document listener it's being cleared.

In my testcase, I see two selection listeners when I get here (https://searchfox.org/mozilla-central/rev/be7d1f2d52dd9474ca2df145190a817614c924e4/dom/base/Selection.cpp#3156), but in the "working" case where we move focus from the address bar and come back to it, there are three listeners.

Looks like both versions register a caret and text editor as a listener, but the working version also registers a nsAccessibilityService.

 I'd suspect that either SetControlSelectionListener fails as Jamie guessed in comment #11 (however it seems was not confirmed in comment #15), or ClearControlSelectionListener is called next to it, or another SetControlSelectionListener is called on top of the original one. But if those stuff work as expected, then the only thing I can think of is we mess up with document lifecycle: for example, when SetControlSelectionListener is set just fine, but then the document accessible is recreated.

(In reply to alexander :surkov (:asurkov) from comment #17)

 I'd suspect that either SetControlSelectionListener fails as Jamie guessed in comment #11 (however it seems was not confirmed in comment #15), or ClearControlSelectionListener is called next to it, or another SetControlSelectionListener is called on top of the original one. But if those stuff work as expected, then the only thing I can think of is we mess up with document lifecycle: for example, when SetControlSelectionListener is set just fine, but then the document accessible is recreated.

Is this what you were getting at before with checking WillRefresh? or is there another place I should keep an eye on to check recreation?

Flags: needinfo?(surkov.alexander)

(In reply to Morgan Reschenberg [:morgan] from comment #18)

(In reply to alexander :surkov (:asurkov) from comment #17)

 I'd suspect that either SetControlSelectionListener fails as Jamie guessed in comment #11 (however it seems was not confirmed in comment #15), or ClearControlSelectionListener is called next to it, or another SetControlSelectionListener is called on top of the original one. But if those stuff work as expected, then the only thing I can think of is we mess up with document lifecycle: for example, when SetControlSelectionListener is set just fine, but then the document accessible is recreated.

Is this what you were getting at before with checking WillRefresh?

right, it all should happen before NotificationController::WillRefresh that triggers SelectionManager::ProcessTextSelChangeEvent. If no appropriate SetControlSelectionListener, then no WillRefresh you'd need to watch.

or is there another place I should keep an eye on to check recreation?

I think I would just log the element in SetControlSelectionListener and log ClearControlSelectionListener. Having another look, it shouldn't be related with document lifecycle, because SelectionManager is a single instance object created per process, and no accessible document is involved until NotifySelectionChanged is triggered (which is not according to your observations).

Flags: needinfo?(surkov.alexander)
Status: NEW → ASSIGNED
Attachment #9124304 - Attachment is obsolete: true
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0a0fe8345747
Reset selection manager to focused element if its frame is recreated. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/65e407ac5e58
Test for a EVENT_TEXT_CARET_MOVED when text is selected in the URL bar of a new window. r=Jamie
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: