Closed Bug 932410 Opened 8 years ago Closed 7 years ago

Allow users to get rid of the focus ring from -moz-appearance:none styled <input type=range>


(Core :: Layout, defect)

Not set





(Reporter: jwatt, Assigned: jwatt)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

Someone has pointed out on my blog that it's not possible to get rid of the focus ring from -moz-appearance:none styled <input type=range>. We should allow that. I think it should be possible by getting the :-moz-focusring context and checking if outline is user-set. If so we should try to use that, but that's probably hard given the current architecture. We could at least check for the "none" value though and skip creating it.
FxOS needs a fix here for bug 973514.
Blocks: 973514
Some background:

Originally I added a focus ring for range using a user-agent style sheet rule along the lines of:

  input[type="range"]:-moz-focusring {
    outline: 1px dotted;

When it came to implementing native theming, this was an issue because we don't want that outline to show if the theme provides its own visual indication of focus. (We don't want two focus rings.)

To handle this I tried in bug 864120 to ignore the -moz-focusring pseudo-class styling (prevent the selector from matching) if native theming was applied. However, as pointed out by bz (in bug 862693 comment 5) that made element state depend on the styles, when styles depend on element state, which isn't right.

Instead, in bug 864120, I removed the -moz-focusring from the UA style sheet and created a new display list item (nsDisplayRangeFocusRing) to paint the focus ring when ranges are not natively styled. The problem with this approach is that the focus ring can't be removed using style, causing this bug and bug 973514.

One way around this bug would be to avoid using nsDisplayRangeFocusRing if 'outline' has been set using something along the lines of HasAuthorSpecifiedRules, but more a HasAnySpecifiedRules (so FxOS can get rid of the outline using a rule in its UA style sheet), for the outline property. dbaron doesn't want to use HasAuthorSpecifiedRules, and especially doesn't want to introduce a HasAnySpecifiedRules extension to that method, though. Instead he would prefer that we kill off nsDisplayRangeFocusRing, add back the original user-agent style sheet rule at the beginning at this comment, and just ignore the 'outline' property for nsRangeFrame if it is natively themed.

For the record I'm not in love with dbaron's approach since it assumes setting of 'outline' equates to -moz-focusring styling. Probably this is fine for virtually all content authors, but I need to think about this some more.
How is this different from the situation buttons are in?  What do we do there?
Button rendering isn't simple (it being done via nsButtonFrameRenderer and involving -moz-focus-inner and -moz-focus-outer pseudo-elements used purely to specify style that nsButtonFrameRenderer will use to paint focus rings) but the short of it is that nsDisplayButtonForeground::Paint only calls nsButtonFrameRenderer::PaintOutlineAndFocusBorders if IsThemed or ThemeDrawsFocusForWidget returns false. So I guess that's essentially doing what dbaron feels we should do for range, except that the user of pseudo-elements means that content can still set an outline independent of the UA style sheet focus rings, which makes me happy.
Attachment #8378033 - Flags: review?(bzbarsky)
Attachment #8378033 - Attachment is obsolete: true
Attachment #8378033 - Flags: review?(bzbarsky)
Attachment #8378036 - Flags: review?(bzbarsky)
With this patch content and UA style sheets would be able to get rid of the UA focus ring by using:

input[type=range]::-moz-focus-outer {
  border: 0;

or otherwise change the style of the focus ring by changing the border values for that pseudo-element.
Comment on attachment 8378036 [details] [diff] [review]
patch to use the same strategy as nsButtonControlFrame

We'll probably need a different approach for standardizing this, but I'm not sure what that should be.  :(

>+nsDisplayRangeFocusRing::GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap)

Should assert in here that styleContext is not null, with a pointer to why (that is, that this display item only exists if it's not null).

>+  if (aBuilder->IsForPainting() ||
>+      !IsVisibleForPainting(aBuilder)) {
>+    // we don't want the focus ring item for hit-testing or if the item isn't
>+    // in the area being [re]painted

Is the IsForPainting() test backwards?  Missing a '!'?

>+    // no ::-moz-outer-focus specified border (how style specifies a focus

::-moz-focus-outer, right?

>+  // called if style changes that would change the -moz-outer-focus


r=me with those nits
Attachment #8378036 - Flags: review?(bzbarsky) → review+


(In reply to Boris Zbarsky [:bz] from comment #8)
> We'll probably need a different approach for standardizing this, but I'm not
> sure what that should be.  :(

Me neither. :(

> Is the IsForPainting() test backwards?  Missing a '!'?

Thanks for catching that - I accidentally removed the check just prior to my last qref, and then screwed up re-adding it when I noticed that reviewing the original patch I attached.
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
this method doesn't work!
outer black border disappear. inner white border is not.
windows 7, ff 43
You need to log in before you can comment on or make changes to this bug.