::-moz-focus-inner should default to padding: 0, border: 0

UNCONFIRMED
Unassigned

Status

()

Core
Layout: Form Controls
UNCONFIRMED
3 years ago
a year ago

People

(Reporter: Alex Bell, Unassigned, NeedInfo)

Tracking

36 Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20141218004002

Steps to reproduce:

Look at a button or input element, and click or tab to give it focus.


Actual results:

There is an inner dotted black focus ring in the default UA style. This is not the normal external CSS outline.


Expected results:

Buttons and inputs should only show the default CSS outline when focused, not an additional internal CSS outline. There seems to be a consensus now among developers that this is an Firefox anomaly that needs to be corrected. See normalize.css :

https://github.com/necolas/normalize.css/blob/master/normalize.css#L311-L315

My very limited understanding is that ::-moz-focus-inner  is simply a hook to allow for native platform styling (though it doesn't do a great job of this in practice). It shouldn't have affiliated non-standard styles.
(Reporter)

Updated

3 years ago
Component: Untriaged → General

Comment 1

3 years ago
I'd say this is basically a duplicate of bug 140562, no?
Component: General → Untriaged
(Reporter)

Comment 2

3 years ago
Well, not quite. I read that entire thread, actually, before posting this, and I think it's a separate idea. That thread is about eliminating the ::-moz-focus-inner entirely. It got no traction from Boris and others because it apparently serves a useful internal purpose. FWIW that entire discussion was also pre-Normalise.

(There's also a lot of obsolete advice in that thread about how setting:

    button::-moz-focus-inner {padding:0; border:0}

will cause the default outline not to display, which hasn't been true for a long time.)

My suggestion here is not to eliminate the property but to change its default. (I think it would also help to expose it in Dev Tools, and I've filed 1112917 for that.)
(Reporter)

Updated

3 years ago
Component: Untriaged → General

Updated

3 years ago
Component: General → Untriaged
Depends on: 140562

Updated

3 years ago
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core

Updated

3 years ago
Component: CSS Parsing and Computation → Layout: Form Controls

Comment 3

a year ago
Created attachment 8816721 [details] [diff] [review]
proposed_forms_css_change.diff

With the benefit of hindsight after working on bug 140562, I think that we can actually get rid of ::moz-focus-inner entirely now. We're only using it on Windows for buttons, where Internet Explorer and Edge seem to use the equivalent of this CSS:

  :moz-focusring {
    outline: 1px dotted;
    outline-offset: -3px;
  }

The only difference between that and what we were doing with ::moz-focus-inner is that our ring is offset a couple of pixels away from the inner content's box, rather than in from the border box of the button. To me that doesn't seem like a big enough difference to warrant keeping ::moz-focus-inner around anymore.

On top of that I doubt there would be major webcompat issues, as ::moz-focus-inner is very limited in what it can actually do (padding and border). CSS reset techniques would also have to be taking :moz-focusring's outline into account as well, so we might as well just use it instead.

Also, I'm not sure if we plan to move away from using :moz-focusring in favor of just using a :focus outline (which is what Chrome/WebKit use), but that's an option here as well, and would be even safer for web-compat.

bz, dbaron, what do you guys think? Anyone else I should ask?
Flags: needinfo?(dbaron)
Flags: needinfo?(bzbarsky)

Comment 4

a year ago
Comment on attachment 8816721 [details] [diff] [review]
proposed_forms_css_change.diff

Ah, right, I somehow forgot that we also have to draw that focusring when -moz-appearance:none is used, meaning that my patch isn't going to cut it. I'll ponder over this a bit more.
Attachment #8816721 - Attachment is obsolete: true
Flags: needinfo?(dbaron)
Flags: needinfo?(bzbarsky)

Comment 5

a year ago
Created attachment 8816739 [details] [diff] [review]
wip.diff

As one might guess, it seems like it would be quite an undertaking to get rid of :moz-focus-inner, but perhaps we could just hide it from web content instead. Chrome lets users over-ride its default button focus rings by simply specifying a value for the button's CSS outline (similarly to how it and Firefox already let the user over-ride default button border/padding/background/etc styles). Perhaps we could do the same.

We could just check if the author has specified any outline styles on the button, then not draw our own focus ring at all. For themes that don't draw their own focus rings (and also the -moz-appearance:none case), this is easy enough. GTK was also fairly easy. I'm new to Cocoa/Obj-C, so I'd need time to figure out that case, but it seems feasible. I'm just not sure about Android/Gonk.

What do you guys think? Is this something we might as well do, given that we're already chipping away at such form-input styling interop issues lately?

(I've attached my current patch in case you'd like to take a quick look).
Flags: needinfo?(dbaron)
Flags: needinfo?(bzbarsky)

Comment 6

a year ago
Created attachment 8817828 [details] [diff] [review]
wip.diff

Just an updated patch with OSX support. Still have to check out Android.
Attachment #8816739 - Attachment is obsolete: true
Hmm.  I seem to recall there were edge cases in which "inset from the button" didn't really match "outset from the text", but maybe that was just a matter of matching old Windows platform behavior?

If we're no longer aiming for that effect, then using :moz-focusring seems to make sense....

If we do use :moz-focusring, what is the issue with "-moz-appearance: none" behavior?
Flags: needinfo?(bzbarsky)
You need to log in before you can comment on or make changes to this bug.