1242715 - [a11y] Add accessibility labels to unlabelled controls.

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

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)

Comment 1

[Patrick Brosset <:pbro>]

Filter on CLIMBING SHOES

Comment 2

[Nancy Pang [:npang]]

Attachment: MozReview Request: Bug 1242715 - Added labels to controls and added test cases. r=pbro

Review commit: https://reviewboard.mozilla.org/r/52958/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52958/

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

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.

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

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?

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

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.

Comment 6

[Nancy Pang [:npang]]

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/

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

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.

Comment 8

[Nancy Pang [:npang]]

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/

Comment 9

[Patrick Brosset <:pbro>]

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.

Comment 10

[Nancy Pang [:npang]]

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/

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ad17f2d6e4f

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7ad17f2d6e4f