Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 2

6 years ago
> (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)

Comment 3

6 years ago
Created attachment 527914 [details] [diff] [review]
Like so
Attachment #527914 - Flags: review?(dbaron)
(Assignee)

Updated

6 years ago
Assignee: nobody → bzbarsky
Priority: -- → P2
Whiteboard: [need review]
Blocks: 267119

Updated

6 years ago
Keywords: dev-doc-needed
(Assignee)

Comment 4

6 years ago
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.
(Assignee)

Comment 6

6 years ago
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+
(Assignee)

Comment 8

6 years ago
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.  :(
(Assignee)

Updated

6 years ago
Whiteboard: [need review] → [need landing]
(Assignee)

Comment 9

6 years ago
Created attachment 528704 [details] [diff] [review]
With that change
(Assignee)

Updated

6 years ago
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]
(Assignee)

Comment 11

6 years ago
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.
(Assignee)

Comment 12

6 years ago
Created attachment 528824 [details] [diff] [review]
This passes try
(Assignee)

Updated

6 years ago
Attachment #528704 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Whiteboard: [need landing]
(Assignee)

Comment 13

6 years ago
Created attachment 547710 [details] [diff] [review]
Merged to tip
(Assignee)

Updated

6 years ago
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
(Assignee)

Updated

6 years ago
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.
(Assignee)

Comment 16

6 years ago
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 ...
(Assignee)

Comment 18

6 years ago
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?
(Assignee)

Comment 20

6 years ago
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.