Rob Campbell [:rc] (:robcee) Reporter
	Description • 10 years ago

when changing focus to the toolbox with the inspector selected, focus is described as "inspector.xul" chrome URL instead of a useful descriptive name.

We should add an aria-label value "inspector pane" or similar for this.

	Athena Assignee
	Comment 1 • 10 years ago

Grabbing

	Marco Zehe (:MarcoZ)
	Comment 2 • 10 years ago

This is the first thing anyone hears when they open the dev tools. It therefore gets a p1.

	Athena Assignee
	Comment 3 • 10 years ago

Looks like none of the tools had a label for the panel, so I went ahead and added a "toolname panel" label to all of them. 

Tested that the panel label is applied for all tools, except for the Scratchpad. I can't figure out how to read out the aria-label for that since it opens its own window.

If there's a future tool added which doesn't have the panel label, then it falls back to the toolname.xul text. This is less than ideal, but alternative was to have it say "undefined panel". So let's just make sure panelLabel is always added in the future!

	Marco Zehe (:MarcoZ)
	Comment 4 • 10 years ago

Athena, was this patch also supposed to fix the label for chrome://browser/content/devtools/framework/toolbox.xul'? If so, it's the only one that doesn't work yet. The others all seem to work fine as far as a quick test shows. Good work!

	Athena Assignee
	Comment 5 • 10 years ago

No, I missed that! Added; thanks for pointing that out.

	Marco Zehe (:MarcoZ)
	Comment 6 • 10 years ago

Comment on attachment 8450050 [details] [diff] [review]
Apply aria-label "Toolname Panel" to all toolbox panels

f=me, thanks!

	Brian Grinstead [:bgrins]
	Comment 7 • 10 years ago

Comment on attachment 8450050 [details] [diff] [review]
Apply aria-label "Toolname Panel" to all toolbox panels

Review of attachment 8450050 [details] [diff] [review]:
-----------------------------------------------------------------

This looks nice.  Please reupload with a commit message with r=bgrins at the end.

Question for testing this out: I'm using VoiceOver for OSX and I notice that on some of the panels it now says things like "Profiler Panel Group" when switching to that panel (great!).  But if there is a control that takes focus immediately, like in the console, it will instead say "text blank".  Which makes sense, but is there a way for it to read off the console header?  It's not a huge deal - I'm sure it will work just the same - but I'd like to make sure I know how to best test accessibility changes out.  I've also tried the Accessibiliy Inspector but for some reason on my local build it only seems to show information about the current Firefox window, not any elements inside of it.

	Marco Zehe (:MarcoZ)
	Comment 8 • 10 years ago

(In reply to Brian Grinstead [:bgrins] from comment #7)
> Question for testing this out: I'm using VoiceOver for OSX and I notice that
> on some of the panels it now says things like "Profiler Panel Group" when
> switching to that panel (great!).  But if there is a control that takes
> focus immediately, like in the console, it will instead say "text blank". 
> Which makes sense, but is there a way for it to read off the console header?

Some screen readers, like NVDA on Windows, will read out the ancestry (parent(s)) panels and their headers/accessible names automatically, with VoiceOver, people will definitely find the panel titles when navigating around the UI with the Ctrl+Option+Arrow key commands that VoiceOver provides.

Note however, that Gecko's support for VoiceOver itself is incomplete, so testing may sometimes yield unexpected results on OS X. I checked the patches on Windows where our implementation is solid.

> It's not a huge deal - I'm sure it will work just the same - but I'd like to
> make sure I know how to best test accessibility changes out.  I've also
> tried the Accessibiliy Inspector but for some reason on my local build it
> only seems to show information about the current Firefox window, not any
> elements inside of it.

The inspector gets the information from the element under the mouse. But for that, each element needs to return its correct bounds. Talk about an incomplete implementation on OS X, you're seeing signs of it right here. :(

So using VoiceOver is still your best bet, or you could install DOM Inspector and look at the Accessibility Tree and see the name of the panel appear in its proper place when walking the Chrome.

	Brian Grinstead [:bgrins]
	Comment 10 • 10 years ago

Comment on attachment 8450215 [details] [diff] [review]
Apply aria-label "x panel" to Toolbox

Review of attachment 8450215 [details] [diff] [review]:
-----------------------------------------------------------------

Changes look great!  I have one suggestion on a string, but beyond that let's land it.  Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=e5ad6b2e31b4

::: browser/locales/en-US/chrome/browser/devtools/toolbox.properties
@@ +36,5 @@
>  toolbox.defaultTitle=Developer Tools
>  
> +# LOCALIZATION NOTE (toolbox.label): This is used as the label for the
> +# toolbox as a whole
> +toolbox.label=Toolbox

I think this would be more descriptive as 'Developer Tools' or 'Developer Tools Toolbox'

	Brian Grinstead [:bgrins]
	Comment 11 • 10 years ago

Actually, now I'm wondering if we should just be reusing toolDefinition.tooltip for the tools for the aria-label.  Of course we would still need a new string for the toolbox itself, but is there some reason we wouldn't want to use the tooltiptext?

	Athena Assignee
	Comment 12 • 10 years ago

Good question. The two considerations I have in mind:

1) I don't know if there's enough context to differentiate the buttons from the panels without explicitly saying "panel". Bug 977262 might help with that.

2) Sme of the tooltip text is almost the same as the button labels, but others are much longer than the proposed panel text, e.g., shader tooltip says "Live GLSL shader language editor for WebGL"

	Marco Zehe (:MarcoZ)
	Comment 14 • 10 years ago

Comment on attachment 8451266 [details] [diff] [review]
label-inspector-xul-bug-977169

Landed on Athena's behalf on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/01356babaefc

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 15 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/01356babaefc