652301 - Focused non-link <a> does not show a focus ring

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

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?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> (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.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 4

[Boris Zbarsky [:bzbarsky]]

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?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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 6

[Boris Zbarsky [:bzbarsky]]

Attachment: slightly more conservative fix

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 8

[Boris Zbarsky [:bzbarsky]]

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.  :(

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: With that change

Comment 10

[Justin Lebar (not reading bugmail)]

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

Comment 11

[Boris Zbarsky [:bzbarsky]]

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.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: This passes try

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Merged to tip

Comment 14

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/7b8b6cc5f662

Comment 15

[Eric Shepherd [:sheppy]]

I'm not seeing anything that obviously requires developer documentation here. Can someone tell me what needs to be written up? Thanks.

Comment 16

[Boris Zbarsky [:bzbarsky]]

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.

Comment 17

[j.j.]

(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 ...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Yes, everything that wasn't in the whitelist before and is not in the blacklist now.

Comment 19

[Eric Shepherd [:sheppy]]

This still doesn't seem like something that requires developer documentation; this is just a cosmetic issue, isn't it?

Comment 20

[Boris Zbarsky [:bzbarsky]]

For web developers, those need documentation...  In particular, we've had a number of web developer bug reports about the new behavior.

Comment 21

[Eric Shepherd [:sheppy]]

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.