Closed Bug 973514 Opened 6 years ago Closed 5 years ago

[Settings] select slider shows dots border in Sound panel

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gasolin, Assigned: jwatt)

References

Details

(Whiteboard: [LibGLA, TD77343, WW, C])

Attachments

(4 files)

When you select slider element in Sound panel, sometimes dots border shows

It's caused by the system default style.

We should hide it because it looks not consistence in this case.

Reproduce step:

1. open Settings > Sound panel
2. select Ringer slider in Volume section, (try to tap the edge of the input element)
3. the  dots border shows

Reproduce Rate:
100%

Expect result:

No dots border shows when select slider
Assignee: nobody → gasolin
Attached image doc_slider.png
Hi omega, does this intended or is just a UI bug?
Flags: needinfo?(ofeng)
It's a bug. Don't show the dots border.
Flags: needinfo?(ofeng)
seems input[type="range"] css not support the `-moz-focus-inner` pseudo-element.
css part
jerry, can you help find related person?
Flags: needinfo?(hshih)
@dbaron kanru point me to reach you with this issue.

I'd try to hide dotted border around element while input[type=range] is selected

tried several combinations:

`input[type=range]::-moz-focus-inner`

`input[type=range]:active::-moz-focus-inner`

`input[type=range]:focus::-moz-focus-inner`

or `input::-moz-focus-inner` https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/wrapper/wrapper.css#L131

with `outline: none` or `border: 0` within it. but all of them not work.

So i wonder if the range input does not support the undocumented `-moz-focus-inner` pseudo-element.

Any suggestion for it?
Flags: needinfo?(hshih) → needinfo?(dbaron)
So from reading the code, -moz-focus-inner is for buttons only. I also find bug 932410 was filed to address this issue.

I think we could render the default focus ring with

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

in forms instead do it in code.
Comment on attachment 8377452 [details] [diff] [review]
Draw input["range"] outline in UA style. Tentative patch

Review of attachment 8377452 [details] [diff] [review]:
-----------------------------------------------------------------

Good detective work to find this code, but removing it will break bug 864120. In other words it will mean that on desktop the styled focus ring will show even when we do not want it to because the native theming gives the visual indication of focus.

I tried to fix the double focus ring issue (without the display list code you're removing) in bug 862693. As noted in that bug, however, we can't decide to ignore the -moz-focusring styling based on whether the element has native theming (see bug 862693 comment 5 and the discussion that followed). Hence why we took the other approach in bug 864120 to display a focus ring using a display list item (instead of a -moz-focusring rule) if native theming isn't handling the focus. The disadvantage of this approach is that content can't prevent the non-themed focus ring from showing.

I'll need to think about what the options are here.
Attachment #8377452 - Flags: feedback?(jwatt)
Attachment #8377452 - Flags: feedback?(dbaron)
Attachment #8377452 - Flags: feedback-
Flags: needinfo?(dbaron)
Probably best that I take this for now.
Assignee: gasolin → jwatt
Depends on: 932410
Whiteboard: [LibGLA, TD77343, WW, C]
Hi, jwatt,
Could you share the status of this issue?
(This issue is reproducing in Flame v2.0 and master as well)
Flags: needinfo?(jwatt)
https://github.com/mozilla-b2g/gaia/pull/23054

There is still an undesirable blue border-bottom on the sliders, but that's a different issue. I tracked it down to https://github.com/jwatt/gaia/blob/4595c734/shared/style/input_areas.css#L216 but since it's not obvious how the gaia team would like to structure their CSS I'll let them pick up and fix that issue in a separate bug.
Flags: needinfo?(jwatt)
Attached file pull request URL
Attachment #8477287 - Flags: review?(ejchen)
Comment on attachment 8477287 [details] [review]
pull request URL

Thanks Jwatt !
Attachment #8477287 - Flags: review?(ejchen) → review+
Anyone know why my pull request hasn't been accepted yet?
Please make sure CI did pass and then you can merge your code on github !
Flags: needinfo?(jwatt)
And also, Jwatt, you can put checkin-needed keyword on "keywords" field and Kyle or someone else would help to merge the code.

I can help to merge by the way. THanks for the works ;)
Thanks, eragonj.
Flags: needinfo?(jwatt)
Keywords: checkin-needed
Jwatt,

I just quickly checked your commit and noticed there are still some failures on CI, can you rebase your patch to latest gaia and force push again ?

https://tbpl.mozilla.org/?rev=9d571376166fddef6332503371e40d51645b352b&tree=Gaia-Try

Thanks.
Flags: needinfo?(jwatt)
There's something rotten with the CSS linter. My use of the px unit with the value "0" triggered a warning (lame - we shouldn't be wasting time looking at trivia warnings like that), but triggering the warning also seems to unblock the reporting of various unrelated and pre-existing CSS errors that were otherwise not being reported. It was this that caused the "Li" failures, not any actual error in my commit.

As for the "Gij" failures, I think something is flaky with that too. This is the result after removing the "px" unit thing. Retriggering "Gij" shows it sometimes failing and sometimes passing:

https://tbpl.mozilla.org/?rev=0df2fc42d931f304fde9fa7ea5b9fb269879b2db&tree=Gaia-Try

Ej, what do you think? Is this good to land now? I'm sure any CI issues are nothing to do with my commit.
Flags: needinfo?(jwatt) → needinfo?(ejchen)
Just checked the Gaia-try and it looks nice. I'll help you merge on master directly !
Flags: needinfo?(ejchen)
Merged into Gaia/master (2.2) : https://github.com/mozilla-b2g/gaia/commit/4e4b87103ffeb6edefb8c8f8b56ea587dbdc9e7f

Thanks Jwatt !
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Thanks for your help, Ej!
You need to log in before you can comment on or make changes to this bug.