The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla8

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

({dev-doc-complete})

Trunk
mozilla8
x86
Mac OS X
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

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.
Created attachment 527914 [details] [diff] [review]
Like so
Attachment #527914 - Flags: review?(dbaron)
Assignee: nobody → bzbarsky
Priority: -- → P2
Whiteboard: [need review]

Updated

6 years ago
Blocks: 267119

Updated

6 years ago
Keywords: dev-doc-needed
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
Attachment #527914 - Attachment is obsolete: true
Attachment #527914 - Flags: review?(dbaron)
Attachment #528124 - Flags: review?(dbaron)
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]
Created attachment 528704 [details] [diff] [review]
With that change
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.
Created attachment 528824 [details] [diff] [review]
This passes try
Attachment #528704 - Attachment is obsolete: true
Whiteboard: [need landing]
Created attachment 547710 [details] [diff] [review]
Merged to tip
Attachment #528824 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/7b8b6cc5f662
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla8
Flags: in-testsuite+
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.

Comment 17

6 years ago
(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.
Keywords: dev-doc-needed → dev-doc-complete

Updated

5 years ago
Depends on: 786428
You need to log in before you can comment on or make changes to this bug.