Closed Bug 1200322 Opened 4 years ago Closed 4 years ago

Bad a11y when pressing Left and Right while the urlbar autocomplete popup is focused

Categories

(Firefox :: Address Bar, defect, P1)

41 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox41 --- fixed
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

(Whiteboard: [fxsearch])

Attachments

(1 file, 2 obsolete files)

1. Type something in the urlbar so that the popup opens with some results.
2. Press the Down arrow key to highlight any result.
3. Press the Left or Right arrow key so that the I-beam in the urlbar moves left or right.

Expected: Your screen reader should read the character under the I-beam as it moves left and right.

Actual: Your screen reader doesn't read anything.

Caused by bug 1174471, and it happens even when UnifiedComplete is disabled.  Commenting out the onKeyPress method in the urlbar binding fixes the problem: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml?rev=14733eceb2ed#189
Flags: firefox-backlog+
Summary: Bad a11y when pressing Left and Right while the urlbar autocomplete is focused → Bad a11y when pressing Left and Right while the urlbar autocomplete popup is focused
The search bar doesn't have the problem described in bug 1174471, interestingly.
Attached patch patch (obsolete) — Splinter Review
This fixes the a11y problem and keeps the fix for bug 1174471 intact.  The problem seems to be not resetting selectedIndex.  That's what nsAutoCompleteController::ClosePopup does: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/autocomplete/nsAutoCompleteController.cpp#1117

ClosePopup is what bug 1174471's original fix was trying to emulate it looks like?  The call from this path here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/autocomplete/nsAutoCompleteController.cpp#589
Assignee: nobody → adw
Status: NEW → ASSIGNED
Attachment #8654996 - Flags: review?(dtownsend)
Attached patch patch v2 (obsolete) — Splinter Review
There should probably be a stopSearch call here too, since the path linked in comment 2 calls ClearSearchTimer before calling ClosePopup.  ClearSearchTimer and StopSearch aren't quite the same thing, but the latter calls the former.
Attachment #8654996 - Attachment is obsolete: true
Attachment #8654996 - Flags: review?(dtownsend)
Attachment #8654999 - Flags: review?(dtownsend)
Attached patch patch v3Splinter Review
Here's an even simpler patch that just resets selectedIndex and then follows the usual nsAutoCompleteController path.  The crux of the problem is selectedIndex not being -1.

Sorry for the bugmail churn, Dave.
Attachment #8654999 - Attachment is obsolete: true
Attachment #8654999 - Flags: review?(dtownsend)
Attachment #8655000 - Flags: review?(dtownsend)
Attachment #8655000 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/mozilla-central/rev/9f7bb8e84057
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Comment on attachment 8655000 [details] [diff] [review]
patch v3

Approval Request Comment
[Feature/regressing bug #]:
Bug 1174471 (landed on 41)

[User impact if declined]:
It will be difficult for people who use screen readers to use the urlbar.

[Describe test coverage new/current, TreeHerder]:
No new tests, urlbar is covered by existing tests, and I manually tested this on Aurora and Beta.

[Risks and why]:
Low risk, simple and well understood change.

[String/UUID change made/needed]:
None
Attachment #8655000 - Flags: approval-mozilla-beta?
Attachment #8655000 - Flags: approval-mozilla-aurora?
Comment on attachment 8655000 [details] [diff] [review]
patch v3

This seems like a very common scenario for accessibility users. Since the patch looks simple enough, let's uplift to Aurora42 and Beta41.
Attachment #8655000 - Flags: approval-mozilla-beta?
Attachment #8655000 - Flags: approval-mozilla-beta+
Attachment #8655000 - Flags: approval-mozilla-aurora?
Attachment #8655000 - Flags: approval-mozilla-aurora+
See Also: → 1327962
You need to log in before you can comment on or make changes to this bug.