Found after bug 490287. The label we mark up as "presentation" still creates an accessible when it shouldn't.

We have common rules for presentation accessible creation, it doesn't depend on it's XUL, HTML. The problem is xul:label is focusable, i.e. nsIContent::IsFocusable returns true since xul:lable implements nsIDOMXULControlElement (http://mxr.mozilla.org/mozilla-central/source/content/xul/content/src/nsXULElement.cpp#568). I think it's wrong. Cc'ing Olli and Neil to get options.

Could we get your opinion guys? It sounds this bug is important for a11y.

Labels shouldn't implement nsIDOMXULControlElement, nor should they be in the tab order. However, accesskeys can be associated with them and clicking them should focus the corresponding control. Perhaps label.focus() should as well as some code might be relying on it, although I wouldn't mind if this wasn't done. This latter part is the only case that would be trickier to handle if labels were not focusable.

Right, we always expose focusable nodes regardless of role=presentation. Like Neil, I say no to tab order. I think the label shouldn't be focusable on its own unless it is possible to perform some action specific to the label. The only other case I think it is OK to focus a label is when it is focused along with it's control (IIRC this happens on IE with checkboxes); but I prefer just the control getting focus. The benefit of allowing clicking on the label to focus the corresponding control is useful for people with targeting issues. I've seen UI where clicking on the label also toggles the control (in the case of toggles); but maybe consistency should rule here. Moving over to XUL, let's make sure we agree there, and fix this at the source.

xulLabel.focus() makes nothing, probably because of focus handling reorg (http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#950 hasn't special processing for XUL labels). If it's a bug then I think it can be fixed in GetRedirectedFocus() (http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#265) I think. I can file bug for this. Note HTML labels has own implementation of focus() method and therefore htmlLabel.focus() works.

Comment on attachment 385626 [details] [diff] [review] wip breaks accesskeys on labels

Comment on attachment 385684 [details] [diff] [review] patch r=me for the tests and the functional aspect. This does what we want, and NVDA now sees no "unknown object" anymore.

Comment on attachment 385684 [details] [diff] [review] patch Let's try this then and see if something breaks. I'm assuming the change to privacypane_tests.js isn't meant to be part of this patch.

Comment on attachment 385684 [details] [diff] [review] patch (In reply to comment #9) > (From update of attachment 385684 [details] [diff] [review]) > Let's try this then and see if something breaks. > > I'm assuming the change to privacypane_tests.js isn't meant to be part of this > patch. No, it is. This mochitest fails with my patch. The problem is they test xul:label as XUL control and I removed it from tests. Let's ask additional review from Ehsan since he is author of this mochitest.

Comment on attachment 385684 [details] [diff] [review] patch Actually, the label should reimplement the disabled property, and the test shouldn't be changed.

(In reply to comment #11) > (From update of attachment 385684 [details] [diff] [review]) > Actually, the label should reimplement the disabled property, and the test > shouldn't be changed. 1. Should this property be a pure XBL property or should it go from IDL interface? 2. Why do we need to have disabled property on the label at all? It makes nothing on the XUL label. 3. Should I reimplement accesskey property on the label? 4. What others properties should I reintroduce on the label (#basecontrol: disabled, tabIndex, #basetext: label, crop, image, command, accesskey)?

(In reply to comment #12) > 1. Should this property be a pure XBL property or should it go from IDL > interface? Just the property is OK. > 2. Why do we need to have disabled property on the label at all? It makes > nothing on the XUL label. Because the label can appear disabled. Also, the click handler checks the disabled property. > 3. Should I reimplement accesskey property on the label? The accessKey property is already implemented on the label binding. A disabled implementation should just go alongside it. > 4. What others properties should I reintroduce on the label (#basecontrol: > disabled, tabIndex, #basetext: label, crop, image, command, accesskey)? None of the others.

> The accessKey property is already implemented on the label binding. A disabled > implementation should just go alongside it. I meant very specifically to put the disabled property on the label implementation ("#text-label"), not the base binding which is also used for <description>

(In reply to comment #15) > I meant very specifically to put the disabled property on the label > implementation ("#text-label"), not the base binding which is also used for > <description> But disabled property is defined in nsIDOMXULDescriptionElement interface (http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/xul/nsIDOMXULDescriptionElement.idl#45).

Comment on attachment 387362 [details] [diff] [review] patch2 OK, leave as is then. The description interface shouldn't really have a disabled property, but that's a separate issue.

landed on 1.9.2 - http://hg.mozilla.org/mozilla-central/rev/1c6fdf03e463

The focus changes here should probably be documented.

Looking at the discussion and code, it looks like what needs documenting here is that labels can't be focused? Or am I missing a nuance here?

(In reply to comment #20) > Looking at the discussion and code, it looks like what needs documenting here > is that labels can't be focused? Or am I missing a nuance here? labels don't implement nsIDOMXULControlElement interface and as consequence are not focusable.

Added a note about this here: https://developer.mozilla.org/en/XUL/label