Closed Bug 1567384 Opened 3 months ago Closed Last month

One-off search buttons no longer get accessibility focus

Categories

(Firefox :: Address Bar, defect, P1)

defect
Points:
1

Tracking

()

RESOLVED FIXED
Firefox 71
Iteration:
71.1 - Sept 2 - 15
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 + fixed
firefox71 --- fixed

People

(Reporter: Jamie, Assigned: dao)

References

(Regression)

Details

(Keywords: access, regression)

Attachments

(1 file)

STR (with the NVDA screen reader):

  1. Focus the address bar.
  2. Type test
  3. Press up arrow.
    • Expected: NVDA should say "Search for "test" with: grouping, Change search settings button"
    • Actual: NVDA says "test" (or nothing without a fix for bug 1567377)

The one-off search code sets aria-activedescendant to fire a11y focus:
https://searchfox.org/mozilla-central/rev/da855d65d1fbdd714190cab2c46130f7422f3699/browser/components/search/content/search-one-offs.js#768

      this.textbox.setAttribute("aria-activedescendant", button.id);

Normally, aria-activedescendant only works on the element with DOM focus. Before bug 1513337, this.textbox referred to the textbox element, which (as a special exception) is considered to have focus when its child input has focus. Now, this.textbox refers to an ancestor hbox element, which does nothing because the hbox doesn't have DOM focus.

This should be fixed by having the one-off search code set aria-activedescendant on the focused input. It's probably also worth adding a test for these one-off buttons to accessible/tests/browser/events/browser_test_focus_urlbar.js.

Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Iteration: --- → 70.1 - Jul 8 - 21
Points: --- → 1
Priority: -- → P1

I'll see how easily I can extend browser_test_focus_urlbar.js for this, but for now I'll just submit the one-line fix.

Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9b1686f9d4c3
Set oneOffSearchButtons.textbox to UrlbarInput::inputField since there's no real textbox anymore. r=adw
Summary: One-of search buttons no longer get accessibility focus → One-off search buttons no longer get accessibility focus

Backed out changeset 9b1686f9d4c3 (bug 1567384) for browser-chrome failures at browser/base/content/test/performance/browser_urlbar_keyed_search.js

Backout: https://hg.mozilla.org/integration/autoland/rev/9723393bae6b61798bad3977567994af94781e02

Failure push: https://hg.mozilla.org/integration/autoland/rev/4db620732a3c1978e0d08148de5341d6a2398fef

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=257435667&repo=autoland&lineNumber=2663

task 2019-07-19T18:55:36.055Z] 18:55:36 INFO - TEST-START | browser/base/content/test/performance/browser_urlbar_keyed_search.js
[task 2019-07-19T18:55:45.039Z] 18:55:45 INFO - TEST-INFO | started process screentopng
[task 2019-07-19T18:55:45.521Z] 18:55:45 INFO - TEST-INFO | screentopng: exit 0
[task 2019-07-19T18:55:45.521Z] 18:55:45 INFO - Buffered messages logged at 18:55:36
[task 2019-07-19T18:55:45.521Z] 18:55:45 INFO - Entering test bound quantumbar
[task 2019-07-19T18:55:45.521Z] 18:55:45 INFO - Buffered messages logged at 18:55:37
[task 2019-07-19T18:55:45.521Z] 18:55:45 INFO - First opening, useAwesomebar=false
[task 2019-07-19T18:55:45.521Z] 18:55:45 INFO - Console message: [JavaScript Warning: "Key event not available on some keyboard layouts: key=“r” modifiers=“accel,alt” id=“key_toggleReaderMode”" {file: "chrome://mochikit/content/tests/SimpleTest/EventUtils.js" line: 870}]
[task 2019-07-19T18:55:45.522Z] 18:55:45 INFO - Console message: [JavaScript Warning: "Key event not available on some keyboard layouts: key=“r” modifiers=“accel,alt” id=“key_quickRestart”" {file: "chrome://mochikit/content/tests/SimpleTest/EventUtils.js" line: 870}]
[task 2019-07-19T18:55:45.522Z] 18:55:45 INFO - Console message: [JavaScript Warning: "Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift” id=“key_browserToolbox”" {file: "chrome://mochikit/content/tests/SimpleTest/EventUtils.js" line: 870}]
[task 2019-07-19T18:55:45.522Z] 18:55:45 INFO - Buffered messages finished
[task 2019-07-19T18:55:45.523Z] 18:55:45 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_keyed_search.js | reflow at __rebuild@chrome://browser/content/search/search-one-offs.js was encountered 2 times,
[task 2019-07-19T18:55:45.525Z] 18:55:45 INFO - it was expected to happen up to 1 times. - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reportUnexpectedReflows :: line 200
[task 2019-07-19T18:55:45.526Z] 18:55:45 INFO - Stack trace:
[task 2019-07-19T18:55:45.528Z] 18:55:45 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reportUnexpectedReflows:200
[task 2019-07-19T18:55:45.529Z] 18:55:45 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:withPerfObserver:701
[task 2019-07-19T18:55:45.531Z] 18:55:45 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:runUrlbarTest:846
[task 2019-07-19T18:55:45.532Z] 18:55:45 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_urlbar_keyed_search.js:quantumbar:47
[task 2019-07-19T18:55:45.535Z] 18:55:45 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1346
[task 2019-07-19T18:55:45.536Z] 18:55:45 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1381
[task 2019-07-19T18:55:45.538Z] 18:55:45 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
[task 2019-07-19T18:55:45.539Z] 18:55:45 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-07-19T18:55:45.544Z] 18:55:45 INFO - Full stack:
[task 2019-07-19T18:55:45.545Z] 18:55:45 INFO - __rebuild@chrome://browser/content/search/search-one-offs.js:503:22
[task 2019-07-19T18:55:45.547Z] 18:55:45 INFO - async*_rebuild@chrome://browser/content/search/search-one-offs.js:441:18
[task 2019-07-19T18:55:45.548Z] 18:55:45 INFO - _on_popupshowing@chrome://browser/content/search/search-one-offs.js:1377:10
[task 2019-07-19T18:55:45.550Z] 18:55:45 INFO - handleEvent@chrome://browser/content/search/search-one-offs.js:175:23
[task 2019-07-19T18:55:45.551Z] 18:55:45 INFO - _openPanel@resource:///modules/UrlbarView.jsm:404:16
[task 2019-07-19T18:55:45.553Z] 18:55:45 INFO - onQueryResults@resource:///modules/UrlbarView.jsm:262:10
[task 2019-07-19T18:55:45.554Z] 18:55:45 INFO - runUrlbarTest/testFn/popup.onQueryResults@chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:776:9
[task 2019-07-19T18:55:45.555Z] 18:55:45 INFO - _notify@resource:///modules/UrlbarController.jsm:576:25
[task 2019-07-19T18:55:45.557Z] 18:55:45 INFO - receiveResults@resource:///modules/UrlbarController.jsm:176:10
[task 2019-07-19T18:55:45.558Z] 18:55:45 INFO - notifyResults@resource:///modules/UrlbarProvidersManager.jsm:388:23
[task 2019-07-19T18:55:45.560Z] 18:55:45 INFO - add@resource:///modules/UrlbarProvidersManager.jsm:394:7
[task 2019-07-19T18:55:45.562Z] 18:55:45 INFO - onSearchResult@resource:///modules/UrlbarProviderUnifiedComplete.jsm:141:13
[task 2019-07-19T18:55:45.563Z] 18:55:45 INFO - notify@resource://gre/modules/UnifiedComplete.jsm:2739:22
[task 2019-07-19T18:55:45.565Z] 18:55:45 INFO - notifyResult@resource://gre/modules/UnifiedComplete.jsm:2754:7
[task 2019-07-19T18:55:45.566Z] 18:55:45 INFO - _addMatch@resource://gre/modules/UnifiedComplete.jsm:2111:10
[task 2019-07-19T18:55:45.567Z] 18:55:45 INFO - _addSearchEngineMatch@resource://gre/modules/UnifiedComplete.jsm:1820:10
[task 2019-07-19T18:55:45.569Z] 18:55:45 INFO - _matchCurrentSearchEngine@resource://gre/modules/UnifiedComplete.jsm:1737:10
[task 2019-07-19T18:55:45.570Z] 18:55:45 INFO - async*_matchFirstHeuristicResult@resource://gre/modules/UnifiedComplete.jsm:1433:32
[task 2019-07-19T18:55:45.572Z] 18:55:45 INFO - asyncexecute@resource://gre/modules/UnifiedComplete.jsm:948:35
[task 2019-07-19T18:55:45.573Z] 18:55:45 INFO - async
startSearch/<@resource://gre/modules/UnifiedComplete.jsm:2896:28
[task 2019-07-19T18:55:45.575Z] 18:55:45 INFO - promise callback*startSearch@resource://gre/modules/UnifiedComplete.jsm:2896:8
[task 2019-07-19T18:55:45.576Z] 18:55:45 INFO - startQuery/<@resource:///modules/UrlbarProviderUnifiedComplete.jsm:150:23
[task 2019-07-19T18:55:45.577Z] 18:55:45 INFO - startQuery@resource:///modules/UrlbarProviderUnifiedComplete.jsm:132:11
[task 2019-07-19T18:55:45.579Z] 18:55:45 INFO - start@resource:///modules/UrlbarProvidersManager.jsm:300:30

Flags: needinfo?(dao+bmo)
Iteration: 70.1 - Jul 8 - 21 → 70.2 - Jul 22 - Aug 4
Flags: needinfo?(dao+bmo)

please file a follow-up bug for test coverage, if not addressing that here.

[Tracking Requested - why for this release]: Accessibility regression

Depends on: 1567377

I notice this now depends on bug 1567377. Just to clarify, aria-activedescendant still needs to go on the input, not the parent role="combobox", as documented in the ARIA spec. You may have had another reason for depending on that bug; please disregard if so.

Duplicate of this bug: 1570482

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

I notice this now depends on bug 1567377. Just to clarify, aria-activedescendant still needs to go on the input, not the parent role="combobox", as documented in the ARIA spec. You may have had another reason for depending on that bug; please disregard if so.

This patch appears to break browser_test_focus_urlbar.js and I suspect that's related to bug 1567377, hence the dependency.

Iteration: 70.2 - Jul 22 - Aug 4 → 70.3 - Aug 5 - 18

browser_test_focus_urlbar.js still times out on "Ensuring autocomplete focus on down arrow (2)" after the second patch from bug 1567377. This failure did not occur when I first attempted to land this: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9b1686f9d4c3608d6fb5be34fadba2460d919f6f

Jamie, any clue what's going on here?

Flags: needinfo?(jteh)

This is a bug in the one-offs code:

  1. The user presses down arrow.
  2. The results list sets aria-activedescendant.
  3. SearchOneOffs._updateStateForButton gets called with null.
  4. _updateStateForButton removes aria-activedescendant, thus overriding 2).

This patch fixes the problem, but you may want to implement this differently:

diff --git a/browser/components/search/content/search-one-offs.js b/browser/components/search/content/search-one-offs.js
index 11cefed6a4efb..9484dd4ceae22 100644
--- a/browser/components/search/content/search-one-offs.js
+++ b/browser/components/search/content/search-one-offs.js
@@ -661,7 +661,10 @@ class SearchOneOffs {

     if (this.textbox) {
       if (!button) {
-        this.textbox.removeAttribute("aria-activedescendant");
+        let active = this.textbox.getAttribute("aria-activedescendant");
+        if (active && active.includes("-engine-one-off-item-")) {
+          this.textbox.removeAttribute("aria-activedescendant");
+        }
       } else {
         this.textbox.setAttribute("aria-activedescendant", button.id);
       }
Flags: needinfo?(jteh)
Depends on: 1577801
Depends on: 1577810
Iteration: 70.3 - Aug 5 - 18 → 71.1 - Sept 2 - 15
Depends on: 1578375
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e41dc7906592
Set oneOffSearchButtons.textbox to UrlbarInput::inputField since there's no real textbox anymore. r=adw
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

Can you request beta uplift? Thanks!

Flags: needinfo?(dao+bmo)

Jamie, can you verify that this is fixed in Nightly? Thanks!

Flags: needinfo?(dao+bmo) → needinfo?(jteh)

This mostly works, except that the first time a one-off search gets selected, it doesn't get a11y focus. Subsequent selections do get a11y focus. STR:

  1. Press alt+d to focus the address bar.
  2. Type: test
  3. Press down arrow repeatedly until you hit the last suggestion.
  4. Press down arrow again.
    • Expected: The first one-off search should get a11y focus.
    • Actual: it doesn't; aria-activedescendant on #urlbar-input is null.
  5. Press down arrow again.
    • Observe: The second one-off search gets a11y focus.
  6. Press escape twice to reset.
  7. Type: test
  8. Press up arrow.
    • Expected: The "Change search settings" button should get a11y focus.
    • Actual: it doesn't; aria-activedescendant on #urlbar-input is null.
Flags: needinfo?(jteh)

OK, Dão, can you file a new bug for the leftover issues?

Flags: needinfo?(dao+bmo)
Blocks: 1580822

Comment on attachment 9079371 [details]
Bug 1567384 - Set oneOffSearchButtons.textbox to UrlbarInput::inputField since there's no real textbox anymore. r=adw

Beta/Release Uplift Approval Request

  • User impact if declined: accessibility is broken for one off search buttons
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: Jamie has verified this, I'm fixing the remaining issue in bug 1580822 where I'm also adding a test.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): trivial fix that can't break accessibility more than it already is
  • String changes made/needed:
Flags: needinfo?(dao+bmo)
Attachment #9079371 - Flags: approval-mozilla-beta?
Flags: in-testsuite+

Comment on attachment 9079371 [details]
Bug 1567384 - Set oneOffSearchButtons.textbox to UrlbarInput::inputField since there's no real textbox anymore. r=adw

Accessibility fix, let's uplift for beta 7.

Attachment #9079371 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.