[a11y] Add accessibility labels to unlabelled controls.

RESOLVED FIXED in Firefox 49

Status

defect
P3
normal
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: yzen, Assigned: npang)

Tracking

(Blocks 1 bug, {access})

unspecified
Firefox 49
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

3 years ago
There are at least 2 controls that are currently not labelled in the inspector view:

* Change position button (after search) 
* Swipe tag hierarchy (an arrow button that moves the hierarchy slider)
Filter on CLIMBING SHOES
Priority: -- → P3
Reporter

Comment 3

3 years ago
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

https://reviewboard.mozilla.org/r/52958/#review49854

::: devtools/client/inspector/test/browser.ini:111
(Diff revision 1)
>  [browser_inspector_navigation.js]
>  [browser_inspector_pane-toggle-01.js]
>  [browser_inspector_pane-toggle-02.js]
>  [browser_inspector_pane-toggle-03.js]
>  [browser_inspector_pane-toggle-04.js]
> +[browser_inspector_pane-toggle-05.js]

I think you can just use the needed part of your test (attribute check) and put it somewhere (where it makes sense in browser_inpsector_pane_toggle-01.js. No need to create a new test for just 1 check

::: devtools/client/inspector/test/browser_inspector_pane-toggle-05.js:15
(Diff revision 1)
> +  info("Open the inspector in a bottom toolbox host");
> +  let {toolbox, inspector} = yield openInspectorForURL("about:blank", "bottom");
> +
> +  let button = inspector.panelDoc.getElementById("inspector-pane-toggle");
> +  ok(button, "The toggle button exists in the DOM");
> +  ok(button.getAttribute("tooltiptext"), "The tool tip is empty");

I think it's best to have a message here that describes a success scenario.
Attachment #8753046 - Flags: review?(yzenevich)
Reporter

Comment 4

3 years ago
The changes look good, thanks! For that particular control. Are you planning on taking care of the other controls that are missing label in this bug or create new ones for those cases?
Assignee

Updated

3 years ago
Assignee: nobody → npang
Status: NEW → ASSIGNED
Assignee

Updated

3 years ago
Attachment #8753046 - Flags: review?(yzenevich)
Attachment #8753046 - Flags: feedback?(yzenevich)
Reporter

Comment 5

3 years ago
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

Removing this as I can only give feedback.
Attachment #8753046 - Flags: review?(yzenevich)
Assignee

Comment 6

3 years ago
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52958/diff/1-2/
Attachment #8753046 - Attachment description: MozReview Request: Added initial side panel label and test (bug 1242715); r?yzen → MozReview Request: Added labels to controls and added test cases (bug 1242715);
Attachment #8753046 - Flags: feedback?(yzenevich) → review?(yzenevich)
Reporter

Comment 7

3 years ago
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

This is really nicely done. Thanks! Could you change the reviewer (in reviewboard) to someone like pbro.
Attachment #8753046 - Flags: review?(yzenevich) → feedback+
Assignee

Comment 8

3 years ago
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52958/diff/2-3/
Attachment #8753046 - Attachment description: MozReview Request: Added labels to controls and added test cases (bug 1242715); → MozReview Request: Added labels to controls and added test cases (bug 1242715); r?pbro
Attachment #8753046 - Flags: feedback+ → review?(pbrosset)
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

https://reviewboard.mozilla.org/r/52958/#review50640

R+ with one string to be updated.

::: devtools/client/locales/en-US/netmonitor.dtd:17
(Diff revision 3)
>    - documentation on web development on the web. -->
>  
> -<!-- LOCALIZATION NOTE (netmonitorUI.perfNotice1/2): These are the labels displayed
> +<!-- LOCALIZATION NOTE (netmonitorUI.perfNotice1/2/3): These are the labels displayed
>    -  in the network table when empty to start performance analysis. -->
>  <!ENTITY netmonitorUI.perfNotice1         "• Click on the">
> -<!ENTITY netmonitorUI.perfNotice2         "button to start performance analysis.">
> +<!ENTITY netmonitorUI.perfNotice2         "Analyze">

Here you are changing the meaning of an existing string. netmonitorUI.perfNotice2 already existed before, but you're changing its value.

See https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Updating_Entity_Names
you should change its name so that localizers know there's a new string to be translated.
Attachment #8753046 - Flags: review?(pbrosset) → review+
Assignee

Updated

3 years ago
Keywords: checkin-needed
Assignee

Comment 10

3 years ago
Comment on attachment 8753046 [details]
MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52958/diff/3-4/
Attachment #8753046 - Attachment description: MozReview Request: Added labels to controls and added test cases (bug 1242715); r?pbro → MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7ad17f2d6e4f
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49

Updated

a year ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.