Last Comment Bug 506554 - Implement the CSS3 pseudo-classes :required and :optional for HTML
: Implement the CSS3 pseudo-classes :required and :optional for HTML
Status: RESOLVED FIXED
parity-Opera
: css3, dev-doc-complete, html5
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- enhancement with 1 vote (vote)
: mozilla2.0b5
Assigned To: Mounir Lamouri (:mounir)
:
: Jet Villegas (:jet)
Mentors:
http://www.w3.org/TR/html5/interactiv...
: 506551 (view as bug list)
Depends on: 345822
Blocks: html5forms 566348
  Show dependency treegraph
 
Reported: 2009-07-26 09:33 PDT by d
Modified: 2010-08-23 12:38 PDT (History)
14 users (show)
roc: blocking1.9.2-
roc: wanted1.9.2-
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
A testcase where both <textarea>'s should have a green background. (457 bytes, text/html)
2009-08-12 07:09 PDT, d
no flags Details
A new testcase which is more "accurate". (533 bytes, text/html)
2009-08-12 07:16 PDT, d
no flags Details
Tests v1 (18.87 KB, patch)
2010-04-30 07:34 PDT, Mounir Lamouri (:mounir)
jonas: review+
Details | Diff | Splinter Review
Patch v1 (9.66 KB, patch)
2010-04-30 07:35 PDT, Mounir Lamouri (:mounir)
jonas: review+
bzbarsky: review+
Details | Diff | Splinter Review
Patch v1.1 (28.96 KB, patch)
2010-07-28 08:40 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Patch v1.2 (26.01 KB, patch)
2010-08-01 12:31 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Removed code (2.25 KB, patch)
2010-08-01 12:40 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Patch v1.3 (28.03 KB, patch)
2010-08-02 10:22 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review

Description d 2009-07-26 09:33:51 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; sv-SE; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729)
Build Identifier: 

Tracking the progress of implementing the CSS3 psuedo-classes :required and
:optional. This discussion at Webkit might be of help:
https://bugs.webkit.org/show_bug.cgi?id=25551

Reproducible: Always
Comment 1 d 2009-07-26 09:34:17 PDT
*** Bug 506551 has been marked as a duplicate of this bug. ***
Comment 2 David Baron :dbaron: ⌚️UTC-8 2009-07-27 05:35:30 PDT
We already implement them in the style system; the question is making particular HTML elements reflect these states when they're supposed to.
Comment 3 d 2009-07-27 08:51:52 PDT
Ok, so does that mean that :required and :optional should be green on this page?

http://en.wikipedia.org/wiki/Comparison_of_layout_engines_%28Cascading_Style_Sheets%29
Comment 4 David Baron :dbaron: ⌚️UTC-8 2009-08-09 12:57:36 PDT
Are there test cases somewhere?  And do you have a pointer to the spec (e.g., HTML5) that says when certain form controls should be in these states?


Also, it generally doesn't work very well when you use the release-blocking nomination flags for features; they're largely used for end-of-the-release-cycle regression fixing (e.g., things that we broke and need to fix before we ship a release, or broken things about already-implemented features that need to be fixed before they cane be shipped), not beginning-of-the-release-cycle decisions about what features to work on.  So requests for new features generally get minused.
Comment 5 d 2009-08-12 06:46:51 PDT
Here we go:

http://www.w3.org/TR/html5/interactive-elements.html#selector-required

Could not find a test case though.

To me it seems like they are optional in almost all conditions except for

" * input elements that are required
  * textarea elements that have a required attribute
"

For example, this <textarea> (that I am writing in) does not have a required attribute. Therefore it matches the selector :optional. If it would have the required attribute, it would match the :required selector.

At least, that's how it seems to me.
Comment 6 d 2009-08-12 07:09:13 PDT
Created attachment 394022 [details]
A testcase where both <textarea>'s should have a green background.

The correct behavior can currently be seen in Opera and Webkit nightlies.
Comment 7 d 2009-08-12 07:16:41 PDT
Created attachment 394025 [details]
A new testcase which is more "accurate".

I figured that the previous textcase would have shown a green background on both textareas even if only the :optional psuedo-class was implemented (as it would ignore the :required part). This still works as expected in Opera and the Webkit nightlies.
Comment 8 d 2009-08-12 08:16:35 PDT
As a bit of a treat/bonus implementing this would make Gecko the first engine to support all selectors (although some are still in experimental stage).
Comment 9 Alex Vincent [:WeirdAl] 2009-09-04 18:24:24 PDT
Sounds like something we should tie to the required property of form controls.  (Which I'm working on.)
Comment 10 Alex Vincent [:WeirdAl] 2009-09-06 02:22:26 PDT
See bug 345624 - that's probably where I'm going to write a patch for this.
Comment 11 Mounir Lamouri (:mounir) 2010-04-30 07:34:58 PDT
Created attachment 442692 [details] [diff] [review]
Tests v1
Comment 12 Mounir Lamouri (:mounir) 2010-04-30 07:35:52 PDT
Created attachment 442693 [details] [diff] [review]
Patch v1
Comment 13 Mounir Lamouri (:mounir) 2010-04-30 07:50:24 PDT
Btw, I'm not sure the :optional pseudo-class has a good specification. Button and select elements can't be required so they are always optional. I do not think having optional buttons is needed. It should even make page authors worse. For select element, they should be able to suffer from type mismatch.

I've open a bug here:
http://www.w3.org/Bugs/Public/show_bug.cgi?id=9626

I will change the patch or open a follow-up depending of the answer I got.
Comment 14 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-04-30 18:26:01 PDT
Comment on attachment 442693 [details] [diff] [review]
Patch v1

I'd like to hear from bz if that is ok perf-wise.

I don't know how often we call IntrinsicState.

To speed things up we could cache the requiredness in a flag.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2010-04-30 18:41:50 PDT
> I don't know how often we call IntrinsicState.

A fair amount, and I plan to make us call it more, but the new code is no slower than existing form control intrinsic state code, right?  When I make us call it more, I'll probably transition the whole thing to a state word, fwiw, or move away from state bits where we get all of them at once or something.

One comment I do have is that I'd prefer "RequiredApplies" to "DoesRequiredApply".
Comment 16 d 2010-05-02 06:31:59 PDT
https://wiki.mozilla.org/User:Mounir.lamouri/HTML5_Forms mentions ":required and :selected". Is that a "mistype", or a for me, unknown pseudo-class?
Comment 17 Mounir Lamouri (:mounir) 2010-05-02 06:42:00 PDT
(In reply to comment #16)
> https://wiki.mozilla.org/User:Mounir.lamouri/HTML5_Forms mentions ":required
> and :selected". Is that a "mistype", or a for me, unknown pseudo-class?

That's a mistake. Even in the summary line I wrote ":optional".
Thanks for reporting it :)
Comment 18 Mounir Lamouri (:mounir) 2010-05-04 02:52:19 PDT
(In reply to comment #15)
> > I don't know how often we call IntrinsicState.
> 
> A fair amount, and I plan to make us call it more, but the new code is no
> slower than existing form control intrinsic state code, right?  When I make us
> call it more, I'll probably transition the whole thing to a state word, fwiw,
> or move away from state bits where we get all of them at once or something.
> 
> One comment I do have is that I'd prefer "RequiredApplies" to
> "DoesRequiredApply".

I used Does${Attribute}Apply everywhere so I prefer to keep that naming. Actually, I don't think it really matters as we are going to remove these functions to use something more flexible to know if a type accepts a specific attribute.
Comment 19 Mounir Lamouri (:mounir) 2010-05-04 02:55:43 PDT
I'm wondering if a sr is really needed here... Olli, do not hesitate to cancel it if you think Jonas and Boris reviews are enough or the patch doesn't worth a super-review.
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2010-05-04 05:21:03 PDT
> I used Does${Attribute}Apply everywhere so I prefer to keep that naming.

Yes, I know you used it everywhere.  And it looks/sounds really weird everywhere to me...  If you prefer to change it throughout, that's fine.

> we are going to remove these

Bug number?

You can treat my review as sr if desired.
Comment 21 Mounir Lamouri (:mounir) 2010-05-04 06:45:25 PDT
(In reply to comment #20)
> > I used Does${Attribute}Apply everywhere so I prefer to keep that naming.
> 
> Yes, I know you used it everywhere.  And it looks/sounds really weird
> everywhere to me...  If you prefer to change it throughout, that's fine.
> 
> > we are going to remove these
> 
> Bug number?

I've just open bug 563637.

> You can treat my review as sr if desired.

Thanks. Then, no need to bother Olli.
Comment 22 d 2010-05-21 07:10:44 PDT
(In reply to comment #2)
> We already implement them in the style system; the question is making
> particular HTML elements reflect these states when they're supposed to.

I know it's quite old now, but I'd like to know; When were these added to the style system, and do they currently "work" on any element at all?
Comment 23 David Baron :dbaron: ⌚️UTC-8 2010-05-21 07:46:09 PDT
They worked on some elements when using the XForms extension.
Comment 24 d 2010-05-21 07:51:10 PDT
(In reply to comment #23)
> They worked on some elements when using the XForms extension.

I see, thanks. When were they added to the style system?
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2010-05-21 08:57:22 PDT
> When were they added to the style system?

http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/layout/style/nsCSSPseudoClassList.h&rev=HEAD has that information, no?  Not sure why you need it, but you don't have to ask for it; it's all on the web.
Comment 26 d 2010-05-21 09:46:20 PDT
(In reply to comment #25)
> > When were they added to the style system?
> 
> http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/layout/style/nsCSSPseudoClassList.h&rev=HEAD
> has that information, no?  Not sure why you need it, but you don't have to ask
> for it; it's all on the web.

Thanks. I'm not experienced enough to know where to look for it, I have no clue what Bonsai is (in this situation), for example, but it did provide me with the information I needed. For the use, it was about giving accurate information for the Wikipedia page: http://en.wikipedia.org/wiki/Comparison_of_layout_engines_%28Cascading_Style_Sheets%29 where support for these psuedo-classes were incorrectly listed as not supported in Gecko.
Comment 27 Boris Zbarsky [:bz] (still a bit busy) 2010-05-21 09:59:05 PDT
Ah, ok.  For HTML purposes they were in fact unsupported, though...

Bonsai's a web interface that lets one access historical CVS information for the mozilla CVS repository.

If you want, I can give you a brief rundown offline on the various webtools you can use to get data of that sort for the Mozilla tree; just let me know by e-mail?
Comment 28 Mounir Lamouri (:mounir) 2010-07-28 08:40:44 PDT
Created attachment 460887 [details] [diff] [review]
Patch v1.1

Updated patch to apply against current tip.
Merged with test patch.
Comment 29 Mounir Lamouri (:mounir) 2010-08-01 12:31:20 PDT
Created attachment 461918 [details] [diff] [review]
Patch v1.2

I've done a cleanup on this patch following a discussion with Boris on IRC which make me realize some code was completely useless.
Comment 30 Mounir Lamouri (:mounir) 2010-08-01 12:40:47 PDT
Created attachment 461920 [details] [diff] [review]
Removed code

Boris, could you confirm this code wasn't needed?

AFAIU, when changing @required IntrinsicState() is going to be called just after updating mAttrsAndChildren then HasAttr(required) will return the new value so nothing should be done in AfterSetAttr except for @type. Right?
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2010-08-01 18:34:47 PDT
That sounds correct, yes.  You should (if we don't have them already) add tests somewhat like this:

  <style>
    input { display: none; }
    input + span { color: red; }
    input[required]:required + span { color: green; }
  </style>
  <input id="x"><span>This should be green</span>
  <script>
    document.body.offsetWidth; 
    document.getElementById("x").setAttribute("required", "required");
  </script>

and a similar test with the colors reversed and removeAttribute.
Comment 32 Mounir Lamouri (:mounir) 2010-08-02 02:19:40 PDT
(In reply to comment #31)
> That sounds correct, yes.  You should (if we don't have them already) add tests
> somewhat like this:

There are already some tests in the patch using setAttribute and removeAttribute for various element/types combination. This is not using the same technique to show the result (text + color) but that's testing the same thing.

Thank you for taking the time to check that change.
Comment 33 Boris Zbarsky [:bz] (still a bit busy) 2010-08-02 07:44:04 PDT
> There are already some tests in the patch using setAttribute

Yes, but none of them are testing the situation where a selector is selecting on both the attribute _and_ the state, which is the situation where the exact way we send state change notifications starts to really matter.  For the tests in the patch, as long as we send them at all things will be fine.
Comment 34 Mounir Lamouri (:mounir) 2010-08-02 10:22:22 PDT
Created attachment 462111 [details] [diff] [review]
Patch v1.3

Adding the tests requested by Boris.
Comment 35 Mounir Lamouri (:mounir) 2010-08-19 14:27:55 PDT
This is now in the tree.
rev: http://hg.mozilla.org/mozilla-central/rev/55ef0e0529bc
Comment 36 Eric Shepherd [:sheppy] 2010-08-23 12:38:04 PDT
Documented:

https://developer.mozilla.org/en/CSS/%3aoptional
https://developer.mozilla.org/en/CSS/%3arequired

And mentioned on:

https://developer.mozilla.org/en/HTML/Element/input

And the Firefox 4 for developers page.

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