Last Comment Bug 84400 - Support :disabled and :enabled pseudo-classes
: Support :disabled and :enabled pseudo-classes
Status: RESOLVED FIXED
[Hixie-PF]
: css3, fixed1.8, testcase
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P3 enhancement with 1 vote (vote)
: Future
Assigned To: Allan Beaufour
: Hixie (not reading bugmail)
:
Mentors:
Depends on: 115462 306614 306620 307249
Blocks: 65133 271720
  Show dependency treegraph
 
Reported: 2001-06-06 17:59 PDT by Skewer
Modified: 2010-03-05 09:30 PST (History)
23 users (show)
mtschrep: blocking1.8b5+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
CSS3 Disabled pseudo-class testcase (1.66 KB, text/html)
2001-06-06 18:00 PDT, Skewer
no flags Details
Patch (12.10 KB, patch)
2005-07-26 04:31 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review
Patch v2 (24.22 KB, patch)
2005-07-26 06:46 PDT, Allan Beaufour
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
Testcase (2.30 KB, application/xhtml+xml)
2005-07-28 02:11 PDT, Allan Beaufour
no flags Details
Updated to tip (24.25 KB, patch)
2005-08-28 20:51 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
Updated to all my comments (24.20 KB, patch)
2005-08-28 20:56 PDT, Boris Zbarsky [:bz] (still a bit busy)
mtschrep: approval1.8b4-
mtschrep: approval1.8b5+
Details | Diff | Splinter Review
Testcase: Controls matching enabled/disabled (5.71 KB, application/xhtml+xml)
2005-09-26 01:00 PDT, Allan Beaufour
no flags Details
Testcase: Controls _not_ matching enabled/disabled (3.26 KB, application/xhtml+xml)
2005-09-26 01:01 PDT, Allan Beaufour
no flags Details
Testcase: Controls matching enabled/disabled v2 (6.59 KB, application/xhtml+xml)
2005-09-27 03:05 PDT, Allan Beaufour
no flags Details
Testcase: Controls _not_ matching enabled/disabled v2 (2.54 KB, application/xhtml+xml)
2005-09-27 03:16 PDT, Allan Beaufour
no flags Details
Simple reftest v1 (2.33 KB, patch)
2007-04-03 03:36 PDT, Ryan Jones
bzbarsky: review-
Details | Diff | Splinter Review
Reftests based on attachment 197532 and attachment 197533 (18.84 KB, patch)
2007-04-11 17:53 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review

Description Skewer 2001-06-06 17:59:33 PDT
Procedure: View the testcase.

Expected: Enabled textbox will be green, disabled textbox will be red.

Actual: :disabled and :enabled pseudo-classes are not implemented.

Build: 2001060604 Win98
Comment 1 Skewer 2001-06-06 18:00:32 PDT
Created attachment 37451 [details]
CSS3 Disabled pseudo-class testcase
Comment 2 Skewer 2001-06-06 18:06:43 PDT
Keywords: testcase, css3
Comment 3 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2002-06-19 21:12:15 PDT
Assigning pierre's remaining Style System-related bugs to myself.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2003-05-03 22:58:21 PDT
So... the hard part here is deciding what these selectors should apply to, no?

So:
 links (note that there is no way to disable these, really)
 form controls

what else?

Also, what's the interaction with -moz-user-focus here?  The text at
http://www.w3.org/TR/2001/CR-css3-selectors-20011113/#UIstates makes me think
that whether these selectors apply depends on whether the element can be
focused, but -mos-user-focus affects that....
Comment 5 Hixie (not reading bugmail) 2003-05-04 08:36:24 PDT
We should implement the proposal in bug 115462 and base this bug's fix on that.
IMHO. But then I would say that, since said proposal is mine...

Hyatt and I have since proposed that for CSS3.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2004-09-22 10:02:51 PDT
So... All HTML nodes can be focusable if a nonnegative tabindex is set.  Per
CSS3, that means they should all match :disabled (or :enabled if tabindex is
indeed set or they're enabled form controls or anchors, etc), right?  See the
"An element is enabled if the user can either activate it or transfer the focus
to it. An element is disabled if it could be enabled, but the user cannot
presently activate it or transfer focus to it" language in the spec...
Comment 7 Hixie (not reading bugmail) 2004-09-23 00:58:00 PDT
tabindex doesn't set if it's focussable, it sets if it is tabbable. (Which can
also imply it's focussable when it might normally not be, but that's another
story.) It seems kinda weird for :disabled to apply to everything, though. Maybe
we should "clarify" this clause in the spec (or alternatively clarify "tabindex"
in a whatwg spec, defining its implications on :disabled).
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2004-09-23 07:08:19 PDT
Yes, some clarification would be nice.  I was all set to implement this, and
then discovered this issue...  I agree that having the world match :disabled is
silly, so some clarity one way or the other would be appreciated.  On the other
hand, what do we mean by "focusable"?  How does this interact with caret
browsing, say?
Comment 9 Hixie (not reading bugmail) 2004-11-21 11:45:59 PST
CSSWG contacted.
http://lists.w3.org/Archives/Member/w3c-css-wg/2004OctDec/0198.html
Comment 10 Anne (:annevk) 2005-04-08 02:28:09 PDT
Hixie: has the WG come to a conclusion about this?
Comment 11 Hixie (not reading bugmail) 2005-04-08 03:59:30 PDT
Not really. I couldn't really convince the CSSWG members that there was an
issue. It was suggested that allowing tabindex="" on everything was wrong.

I guess we'll have to define this in the WHATWG spec instead. I'd say things
should be defined something as follows: "An element is enabled if the user can
either activate it or transfer the focus to it. An element is disabled if its
enabled state has been explicitly overriden."

If you can find better wording I'm all for it. I hope to just list the relevant
attributes in the WHATWG spec. ("This makes it enabled, this makes it disabled,
this makes it nothing at all, ..." etc.)
Comment 12 Allan Beaufour 2005-07-26 04:31:16 PDT
Created attachment 190540 [details] [diff] [review]
Patch

I just check for the presence of the disabled attribute, like stated in:
http://whatwg.org/specs/web-forms/current-work/#relation

Notes:
* does not check ancestors

* matches _all_ elements inheriting from nsGenericHTMLFormElement
  Per the WebForms 2 spec. the only exception is output, which we do not have
(yet)
Comment 13 Allan Beaufour 2005-07-26 05:43:40 PDT
Comment on attachment 190540 [details] [diff] [review]
Patch

(In reply to comment #12)
> Created an attachment (id=190540) [edit]
>
> * matches _all_ elements inheriting from nsGenericHTMLFormElement
>   Per the WebForms 2 spec. the only exception is output, which we do not have
> (yet)

Ach. From Web Forms 2: "The term form control refers to input, output, select,
textarea and button", so I'll need to check that.
Comment 14 Allan Beaufour 2005-07-26 06:46:58 PDT
Created attachment 190549 [details] [diff] [review]
Patch v2

Ok this adds nsGenericHTMLFormElement::IsFormControlOrFieldSet(),
used by AfterSetAttr() and GetIntrinsicState() (forcing GetType() to be const,
which makes sense anyway).

So the only thing that this patch does not handle is checking for "disabled by
inheritance"
Comment 15 Anne (:annevk) 2005-07-27 03:36:38 PDT
Some testcases:
 <http://annevankesteren.nl/test/css/ui/006>
 <http://annevankesteren.nl/test/css/ui/007>

As we do not do inheritence yet the latter should probably show no green at all.
Comment 16 Allan Beaufour 2005-07-28 02:11:30 PDT
Created attachment 190821 [details]
Testcase
Comment 17 Allan Beaufour 2005-08-04 01:45:58 PDT
(In reply to comment #14)
> Created an attachment (id=190549) [edit]
> Patch v2
> 
> Ok this adds nsGenericHTMLFormElement::IsFormControlOrFieldSet(),
> used by AfterSetAttr() and GetIntrinsicState() (forcing GetType() to be const,
> which makes sense anyway).
> 
> So the only thing that this patch does not handle is checking for "disabled by
> inheritance"

Which btw is WF2 only, as the disabled attribute is not defined for fieldsets in
HTML4
Comment 18 Anne (:annevk) 2005-08-04 02:11:59 PDT
Correct. Form controls inside a disabled fieldset element are not implicitly
disabled in Mozilla. Perhaps this is something for 1.9 if we are doing WF2 by then.
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2005-08-04 13:24:26 PDT
Same question here as in bug 302188 comment 21.
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 20:45:35 PDT
Comment on attachment 190549 [details] [diff] [review]
Patch v2

>Index: content/html/content/src/nsGenericHTMLElement.h

>+  /**
>+   * Called when an attribute has just been changed.
>+   *
>+   * Note that the function is also called if the attribute change fails.

s/the function/this function/

r+sr=bzbarsky with that.  I'll try to check this in in the next day or two.
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 20:51:38 PDT
Comment on attachment 190549 [details] [diff] [review]
Patch v2

Though this would have been much easier to review if there were a diff -w
version attached as well...
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 20:51:46 PDT
Created attachment 194143 [details] [diff] [review]
Updated to tip
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 20:53:14 PDT
Comment on attachment 194143 [details] [diff] [review]
Updated to tip

>Index: content/events/public/nsIEventStateManager.h
>+nsGenericHTMLFormElement::IsFormControlOrFieldSet() const
>+  if (type == NS_FORM_LABEL ||
>+      type == NS_FORM_OPTION ||

This could just return the right boolean, I just realized... I'll make that
change.
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 20:56:41 PDT
Created attachment 194145 [details] [diff] [review]
Updated to all my comments
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 21:07:00 PDT
Anne, may I check your testcases into the layout regression tests?

Other tests I'd kinda like to see here:

1)  Tests ensuring that random elements match neither :disabled nor :enabled
2)  Tests ensuring that all the form controls that should match do (not just
    <input>).
3)  Tests ensuring that form controls that don't match do not.

The full list of form control types is found at
http://lxr.mozilla.org/seamonkey/source/content/html/content/public/nsIFormControl.h#49
if that will help.

Note that none of the tests should require user interaction to run, since I want
to add them to the automated regression tests.
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2005-08-28 21:08:12 PDT
"kinda like to see" means "will need to write before I land this unless someone
else gets there first"...
Comment 27 Allan Beaufour 2005-08-29 10:21:25 PDT
(In reply to comment #21)
> (From update of attachment 190549 [details] [diff] [review] [edit])
> Though this would have been much easier to review if there were a diff -w
> version attached as well...

:( I actually promised you that on one of the other bugs, didn't I? I forgot all
about it, sorry for that.
Comment 28 Boris Zbarsky [:bz] (still a bit busy) 2005-08-29 21:44:20 PDT
I checked in the patch.  Still waiting on Anne to find out whether I can check
in the tests he wrote as regression tests.

Allan, I assume you want to try for 1.8 approval for this?
Comment 29 Allan Beaufour 2005-08-30 09:54:23 PDT
(In reply to comment #28)
> I checked in the patch.

Thanks!

> Allan, I assume you want to try for 1.8 approval for this?

Yes.
Comment 30 Boris Zbarsky [:bz] (still a bit busy) 2005-08-30 10:36:54 PDT
Comment on attachment 194145 [details] [diff] [review]
Updated to all my comments

Requesting 1.8 approval.  This should be pretty safe, and very useful for
XForms...
Comment 31 Mike Schroepfer 2005-08-31 11:45:51 PDT
Comment on attachment 194145 [details] [diff] [review]
Updated to all my comments

Locking down for beta - not taking any new features at this point.
Comment 32 Allan Beaufour 2005-08-31 14:41:14 PDT
(In reply to comment #25)
> Anne, may I check your testcases into the layout regression tests?
> 
> Other tests I'd kinda like to see here:
> 
> 1)  Tests ensuring that random elements match neither :disabled nor :enabled
> 2)  Tests ensuring that all the form controls that should match do (not just
>     <input>).
> 3)  Tests ensuring that form controls that don't match do not.

I've created some tests here:
http://beaufour.dk/tmp/
I need to check option, optgroup, and legend, but the rest is there I think.

The "matching enabled/disabled" one exibits weird behaviour on reload, but
that's because of form restoration. I'm not sure it's entirely correct behaviour
(see bug 306616).
Comment 33 Allan Beaufour 2005-09-01 12:19:48 PDT
(In reply to comment #32)
> I've created some tests here:
> http://beaufour.dk/tmp/
> I need to check option, optgroup, and legend, but the rest is there I think.

Included the rest now.
Comment 34 Anne (:annevk) 2005-09-03 22:54:16 PDT
(In reply to comment #25)
> Anne, may I check your testcases into the layout regression tests?

Yes. (I will try to make more testcases later using your guidelines. Might take
some weeks though.)
Comment 35 Boris Zbarsky [:bz] (still a bit busy) 2005-09-06 12:09:46 PDT
Hmm.. This caused bug 307249
Comment 36 Boris Zbarsky [:bz] (still a bit busy) 2005-09-06 23:11:57 PDT
Note that the regressions and followups (the bugs blocking this one with 3xxxxx
numbers) need to be addressed before we have a good idea of how safe this patch
is for branch.
Comment 37 Asa Dotzler [:asa] 2005-09-14 14:15:06 PDT
Comment on attachment 194145 [details] [diff] [review]
Updated to all my comments

please rerequest approval when this is in better shape. Thanks.
Comment 38 Allan Beaufour 2005-09-26 01:00:37 PDT
Created attachment 197389 [details]
Testcase: Controls matching enabled/disabled
Comment 39 Allan Beaufour 2005-09-26 01:01:18 PDT
Created attachment 197390 [details]
Testcase: Controls _not_ matching enabled/disabled
Comment 40 Mike Schroepfer 2005-09-26 14:34:42 PDT
Comment on attachment 194145 [details] [diff] [review]
Updated to all my comments

Approved per 9/26 bug triage.  Please make sure to land the regression fixes at
the same time.
Comment 41 Allan Beaufour 2005-09-27 03:03:57 PDT
Oops, uploaded old versions of testcases that do not incorporate bug 306620. New
ones one the way.
Comment 42 Allan Beaufour 2005-09-27 03:05:44 PDT
Created attachment 197532 [details]
Testcase: Controls matching enabled/disabled v2
Comment 43 Allan Beaufour 2005-09-27 03:16:31 PDT
Created attachment 197533 [details]
Testcase: Controls _not_ matching enabled/disabled v2
Comment 44 Allan Beaufour 2005-09-27 03:29:49 PDT
checked in to 1.8 branch
Comment 45 Boris Zbarsky [:bz] (still a bit busy) 2006-02-10 13:17:19 PST
Did the testcases ever get added to the regression tests?  If not, they should be.  Please do that.  That includes the testcases Anne has posted links to in the bug, since he's said he's ok with adding them to the regression tests.
Comment 46 Allan Beaufour 2006-03-08 04:35:19 PST
(In reply to comment #45)
> Did the testcases ever get added to the regression tests?  If not, they should
> be.  Please do that.  That includes the testcases Anne has posted links to in
> the bug, since he's said he's ok with adding them to the regression tests.

I've just checked them in.
Comment 47 Ryan Jones 2007-04-03 03:36:02 PDT
Created attachment 260441 [details] [diff] [review]
Simple reftest v1
Comment 48 Boris Zbarsky [:bz] (still a bit busy) 2007-04-03 09:08:33 PDT
I probably won't get to this until a week or two from now.
Comment 49 Boris Zbarsky [:bz] (still a bit busy) 2007-04-11 17:27:46 PDT
Comment on attachment 260441 [details] [diff] [review]
Simple reftest v1

That doesn't so much test anything, since the color doesn't apply to anything in those testcases....
Comment 50 Boris Zbarsky [:bz] (still a bit busy) 2007-04-11 17:53:33 PDT
Created attachment 261322 [details] [diff] [review]
Reftests based on attachment 197532 [details] and attachment 197533 [details]
Comment 51 Boris Zbarsky [:bz] (still a bit busy) 2007-04-11 17:54:11 PDT
Checked in those tests.

Note You need to log in before you can comment on or make changes to this bug.