Closed Bug 1588416 Opened 5 years ago Closed 5 years ago

One-off search grouping no longer labelled for accessibility

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

(Regression)

Details

(Keywords: access, regression, Whiteboard: [access-p2])

Attachments

(1 file)

STR (with the NVDA screen reader):

  1. Press alt+d to focus the address bar.
  2. Type: test
  3. Press up arrow.
  4. Press up arrow again.
    • Expected: NVDA should say, "This time, search with: grouping, Amazon.com.au button" (amazon.com.au might be different depending on available search engines)
    • Actual: NVDA says, "Amazon.com.au button" (no mention of the grouping)

This is because the grouping (hbox.search-panel-one-offs) no longer has an accessibility label.

11:54.78 INFO: Last good revision: 60d9f8a1264118769a4903adba460a2505cd66a3
11:54.78 INFO: First bad revision: b124b26983e4ed57a994bb11a590f9d60447e478
11:54.78 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=60d9f8a1264118769a4903adba460a2505cd66a3&tochange=b124b26983e4ed57a994bb11a590f9d60447e478

This implicates bug 1563026.

Wow. This took me forever to track down.

The issue is that the code assumes the header label will have a .value property, but after the shift to Fluent, it doesn't:

this.buttons.setAttribute("aria-label", headerText.value);

https://searchfox.org/mozilla-central/rev/6866d3a650c826f1cabd123663e84b95ee743701/browser/components/search/content/search-one-offs.js#472

The easy fix is to change .value to .textContent. However, the more correct fix is to set an id on headerText (using telemetryOrigin for uniqueness; see _buttonIDForEngine) and then set aria-labelledby on this.buttons to this id.

We should also add a test for this. In accessible/tests/browser/events/browser_test_focus_urlbar.js, we can change the isEventForOneOffButton function to test if the parent accessible has a role of grouping and has a label:

function isEventForOneOffButton(event) {
  let parent = event.accessible.parent;
  return (
    event.accessible.role == ROLE_PUSHBUTTON &&
    parent &&
    parent.role == ROLE_GROUPING &&
    parent.name
  );
}
Whiteboard: [access-p2]

The previous code relied on the header label having a value property.
However, after the conversion to Fluent in bug 1563026, this is no longer the case.
Instead, use aria-labelledby, which avoids label duplication and will work irrespective of the label's implementation.

Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df25bc388094
Reinstate the label for the search one-offs container. r=Standard8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
Assignee: nobody → jteh
QA Whiteboard: [qa-71b-p2]
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: