973514 - [Settings] select slider shows dots border in Sound panel

Status: RESOLVED FIXED

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

Description

[[e-mail]]

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

Comment 1

[[e-mail]]

Attachment: doc_slider.png

Comment 2

[[e-mail]]

Hi omega, does this intended or is just a UI bug?

Comment 3

[Omega Feng [:Omega] [:馮於懋]]

It's a bug. Don't show the dots border.

Comment 4

[[e-mail]]

seems input[type="range"] css not support the `-moz-focus-inner` pseudo-element.

Comment 5

[[e-mail]]

Attachment: pull request redirect to github

css part

Comment 6

[[e-mail]]

jerry, can you help find related person?

Comment 7

[[e-mail]]

@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?

Comment 8

[Kan-Ru Chen [:kanru] (UTC+9)]

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 9

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Draw input["range"] outline in UA style. Tentative patch

Comment 10

[Jonathan Watt [:jwatt]]

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.

Comment 11

[Jonathan Watt [:jwatt]]

Probably best that I take this for now.

Comment 12

[hansu9866]

Hi, jwatt,
Could you share the status of this issue?
(This issue is reproducing in Flame v2.0 and master as well)

Comment 13

[Jonathan Watt [: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.

Comment 14

[Jonathan Watt [:jwatt]]

Attachment: pull request URL

Comment 15

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8477287 [details] [review]
pull request URL

Thanks Jwatt !

Comment 16

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Just filed one more bug for this in https://bugzilla.mozilla.org/show_bug.cgi?id=1057917, thanks Jwatt !

Comment 17

[Jonathan Watt [:jwatt]]

Anyone know why my pull request hasn't been accepted yet?

Comment 18

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Please make sure CI did pass and then you can merge your code on github !

Comment 19

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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 ;)

Comment 20

[Jonathan Watt [:jwatt]]

Thanks, eragonj.

Comment 21

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

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.

Comment 22

[Jonathan Watt [: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.

Comment 23

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Just checked the Gaia-try and it looks nice. I'll help you merge on master directly !

Comment 24

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Merged into Gaia/master (2.2) : https://github.com/mozilla-b2g/gaia/commit/4e4b87103ffeb6edefb8c8f8b56ea587dbdc9e7f

Thanks Jwatt !

Comment 25

[Jonathan Watt [:jwatt]]

Thanks for your help, Ej!