872351 - display:none does not work with <input type=range> on FF21

Status: RESOLVED WONTFIX

(Core :: Layout: Form Controls, defect)

Description

[Chris Adams]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_3) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.65 Safari/537.31

Steps to reproduce:

I set display:none on an <input type=range> control:

http://jsfiddle.net/acdha/pvNV3/embedded/result/


Actual results:

The input remained visible


Expected results:

It should have been hidden

Comment 1

[Gordon P. Hemsley [:GPHemsley]]

This works as expected with builds that support <input type=range> (like Aurora 22), but this is apparently a (p)regression with earlier builds (like the newly-released Firefox 21).

Comment 2

[Boris Zbarsky [:bzbarsky]]

Actually, Aurora 22 does not support <input type="range">.

But it looks like we never landed that patch to disable in 21, so we shipped a rule like this in forms.css:

  input[type="range"] { display: inline-block !important; }

This went away in the 22 cycle in bug 840820...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Oh and we never landed bug 859868 on 21?  :(

Comment 4

[Boris Zbarsky [:bzbarsky]]

Seems like we should take some sort of fix for this as a ridealong if we ship any 21.x updates, if we can.

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: full backout of all 7 range patches that landed before v21 uplift

Uff. For whatever reason, I had it in my head that I didn't start landing range patches until after the v21 uplift--it wasn't even something I thought to question or check. :(

For reference, this would be a compressed backout of all 7 of the range patches that landed before the v21 uplift, which are:

  Bug 842158 (16ff7211d7b2)
  Bug 842158 (841077d27df8)
  Bug 838256 (87d5ffb1da72)
  Bug 838256 (5e660787834c)
  Bug 838256 (7db39954cc10)
  Bug 836323 (fb7e4cc68108)
  Bug 836314 (125640d90631)

A lot of this is removal of tests, but it's still a non-trivial patch.

Comment 6

[Jonathan Watt [:jwatt]]

Man, these landed on the 16th and 17th of February, and the uplift was on the 18th...stupid.

Comment 7

[Jonathan Watt [:jwatt]]

Oh, I know why, because at that point I was still thinking everything was safely behind a pref. It wasn't until shortly before filing bug 859868 in early April that I pushed to try and noticed that the forms.css changes cause failures...and by that time I forgot I'd started pushing a day or two before the v21 uplifts, not after.

Comment 8

[Jonathan Watt [:jwatt]]

The equivalent of the patch for bug 859868 for v21:

https://tbpl.mozilla.org/?tree=Try&rev=0c88a4bca22b

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: more realistic patch

I can't be completely sure that this will pass try since there was a huge amount of churn on the range code in the six weeks between v21 and v22 uplift. Might as well get review to make sure that this is ready as soon as possible assuming Try passes.

Comment 10

[Alex Keybl [:akeybl]]

Is this tracking nomination a request to take bug 859868 in FF21 if we ship a 21.0.1?

Comment 11

[Boris Zbarsky [:bzbarsky]]

We should probably take the patch in this bug instead....

Comment 12

[Jonathan Watt [:jwatt]]

Comment on attachment 749811 [details] [diff] [review]
more realistic patch

Just to be clear, this only shipping changes here are the changes to forms.css to remove the input[type=range] styling. The rest is all test fixes to take account of running tests with <input type=range> turned off.

[Approval Request Comment]
Regression caused by (bug #): bug 838256
User impact if declined: webpages can't hide <input type=range>, which may break some libraries that hide our non-implementation and replace it with a JS version.
Testing completed (on m-c, etc.): same changes landed in v22
Risk to taking this patch (and alternatives if risky): should be low
String or IDL/UUID changes made by this patch: none

Comment 13

[bhavana bajaj [:bajaj]]

I am going to track this and may consider it as a potential ride-along if we had a Fx21 dot release due to correctness here.It will also be good to add any additional information of any known web-compat issues due to this bug for Fx21 cycle if known which will really help here.

Comment 14

[Chris Adams]

For https://github.com/freqdec/fd-slider/issues/48 I tested and it appears that the least impact would require a JavaScript change but it could be a fairly minimal chunk of JavaScript: 

    if (input.type != "range" && input.getAttribute("type") == "range") {
        input.type = "number";
    }

I haven't found a pure-CSS solution for this, unfortunately, but this is a small DOM modification and it doesn't require a browser version test.