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:
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.
data:text/html,<div style="overflow: auto"><div style="outline: 10px solid black"><br><br><br><br><br><br><br>
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.
Created attachment 527914 [details] [diff] [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.
Created attachment 528124 [details] [diff] [review]
slightly more conservative fix
Comment on attachment 528124 [details] [diff] [review]
slightly more conservative fix
r=dbaron, though I might have written:
Hopefully the tests won't have reliability issues.
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. :(
Created attachment 528704 [details] [diff] [review]
With that change
and caused orange. So a bustage fix was applied:
But there was still orange, so khuey backed the whole thing out:
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.
Created attachment 528824 [details] [diff] [review]
This passes try
Created attachment 547710 [details] [diff] [review]
Merged to tip
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:
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.