Closed Bug 1242715 Opened 5 years ago Closed 4 years ago

[a11y] Add accessibility labels to unlabelled controls.

Categories

(DevTools :: Inspector, defect, P3)

defect

Tracking

(firefox49 fixed)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed

People

(Reporter: yzen, Assigned: npang)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

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
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)
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: nobody → npang
Status: NEW → ASSIGNED
Attachment #8753046 - Flags: review?(yzenevich)
Attachment #8753046 - Flags: feedback?(yzenevich)
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)
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)
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+
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+
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/7ad17f2d6e4f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.