Closed Bug 652301 Opened 10 years ago Closed 10 years ago
Focused non-link <a> does not show a focus ring
It looks like we have two selectors in our UA stylesheet that show focus rings. ua.css has "*|*:-moz-any-link:-moz-focusring" while html.css has a long list of element names _not_ including "a". So given this markup: <a tabindex="0">anchor</a> I can tab to the anchor and it gets focus but does not draw a focus ring. This seems undesirable. Is there a reason not to add "a" to the list in html.css? And should we really be restricting that list to certain HTML elements?
Now that outlines don't cause scrollable overflow (right?) I think there's less risk of breakage from allowing more things to draw focus rings. We probably should.
> (right?) data:text/html,<div style="overflow: auto"><div style="outline: 10px solid black"><br><br><br><br><br><br><br> says yes. The only real risk is adding another *|*:something selector, but we'd be removing one too (the :-moz-any-link one), so maybe it's ok.
Assignee: nobody → bzbarsky
Priority: -- → P2
Whiteboard: [need review]
This is no good, sadly... in particular, it makes us put focus outlines on <input> (which we did not before) and some XUL stuff (so e.g. the text in XUL alerts gets focus outline, the text input inside the url bar gets a focus outline, etc). I suppose I could restrict the outlining to links and HTML elements except input, button, select, textarea. Or I could just add "a" to the list and have random tag names not work. David, preferences?
Obviously adding a to the list is the safest, but trying to get all HTML elements except for the known one we shouldn't do sounds worth a try.
Comment on attachment 528124 [details] [diff] [review] slightly more conservative fix r=dbaron, though I might have written: :-moz-focusring:not(input):not(button):not(select):not(textarea) Hopefully the tests won't have reliability issues.
Attachment #528124 - Flags: review?(dbaron) → review+
Hmm. I'll switch to that :not(a):not(b) pattern, yes. The tests do need the reftest window to have focus, but so do other existing reftests. :(
Whiteboard: [need review] → [need landing]
Attachment #528124 - Attachment is obsolete: true
This landed: http://hg.mozilla.org/mozilla-central/rev/72430b4913e4 and caused orange. So a bustage fix was applied: http://hg.mozilla.org/mozilla-central/rev/12f371cb3c7e But there was still orange, so khuey backed the whole thing out: http://hg.mozilla.org/mozilla-central/rev/1274b3268a02 http://hg.mozilla.org/mozilla-central/rev/c5813f787648 http://hg.mozilla.org/mozilla-central/rev/7cc12a215a3b http://hg.mozilla.org/mozilla-central/rev/02c2e04f09cf
Whiteboard: [need landing]
OK, looks like in addition to frame and iframe I need body and html.... Pushed and updated patch to try; will attach if it cycles green.
Attachment #528704 - Attachment is obsolete: true
Attachment #528824 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla8
I'm not seeing anything that obviously requires developer documentation here. Can someone tell me what needs to be written up? Thanks.
The main change that needs documenting is that now unknown HTML elements that are made focusable through the use of tabindex will now show a focus outline. They didn't use to do that.
(In reply to Boris Zbarsky (:bz) from comment #16) > The main change that needs documenting is that now unknown HTML elements ... including all HTML5 elements ...
Yes, everything that wasn't in the whitelist before and is not in the blacklist now.
This still doesn't seem like something that requires developer documentation; this is just a cosmetic issue, isn't it?
For web developers, those need documentation... In particular, we've had a number of web developer bug reports about the new behavior.
I've added documentation explaining this here: https://developer.mozilla.org/en/Focus_management_in_HTML Please let me know if this is inadequate and feel free to augment if more is needed. This is also mentioned on Firefox 8 for developers.
You need to log in before you can comment on or make changes to this bug.