Closed
Bug 932410
Opened 11 years ago
Closed 11 years ago
Allow users to get rid of the focus ring from -moz-appearance:none styled <input type=range>
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: jwatt, Assigned: jwatt)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 2•11 years ago
|
||
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.
Comment 3•11 years ago
|
||
How is this different from the situation buttons are in? What do we do there?
Assignee | ||
Comment 4•11 years ago
|
||
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.
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8378033 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #8378033 -
Attachment is obsolete: true
Attachment #8378033 -
Flags: review?(bzbarsky)
Attachment #8378036 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 7•11 years ago
|
||
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 8•11 years ago
|
||
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
-moz-focus-outer?
r=me with those nits
Attachment #8378036 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd1bb1422d60
Thanks!
(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.
Comment 10•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment 11•9 years ago
|
||
this method doesn't work!
https://jsfiddle.net/CoolCmd/41av01Lg/
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.
Description
•