410472 - :enabled/:disabled pseudo classes should not match inputs of type "hidden"

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Patch and tests coming up shortly.

Comment 1

[Martijn Wargers (dead)]

Why? Because of the text here?
http://www.w3.org/TR/css3-selectors/#UIstates

Comment 2

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch

(In reply to comment #1)
> Why? Because of the text here?
> http://www.w3.org/TR/css3-selectors/#UIstates
> 
Yep, exactly.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 295176 [details] [diff] [review]
Patch

This isn't the place to fix it.  It just slows down this code, and that patch is wrong anyway (e.g. I don't see you checking anywhere that you're working with an HTML <input>).  The right place to fix this is in the relevant IntrinsicState() implementation.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I should also note that the definition cited in comment 1 is somewhat silly.  For example, you can't tell whether you can "either activate it or transfer the focus to it" until _after_ you have resolved style.  Consider:

  input[type="text"] { display: none; }

Now inputs can't be focused or activated.  That can't stop them from matching :enabled/:disabled, of course.

So it might be worth raising the question of whether hidden inputs should match these selectors in .layout or possibly [e-mail].

Comment 5

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v2

I agree the wording's a bit odd. Seems the intent is to have these selectors apply to any control capable of accepting user input, be it currently rendered or not - excluding hidden inputs since they don't directly accept input and shouldn't really ever have a UI state.
I'll follow up with Hixie/www-style to clarify this though.

Comment 6

[Martijn Wargers (dead)]

Fwiw, if that's the intent, I don't think I agree with that.
<input type="hidden" disabled> prevents the corresponding name/value pair to get submitted, so a hidden input can be disabled.
It would mean that html disabled != css :disabled.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 295191 [details] [diff] [review]
Patch v2

Much better, but this messes up dynamic type changes.  See the comment at <http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/html/content/src/nsGenericHTMLElement.cpp&rev=1.744&mark=2813-2814#2811>.  With this change, that comment becomes false.  You should be able to write a test that fails with this patch based on triggering that codepath....  Basically, have a rule like "input:enabled + div { color: green; }" and have a text input followed by a div, then change the input's type to hidden.  You could even make the text input display:none so the div is the only thing showing up before/after.

Comment 8

[Hixie (not reading bugmail)]

I agree that this isn't well defined and will remove the check from the Acid3 test.