Closed Bug 302186 Opened 19 years ago Closed 18 years ago

Support :default pseudoclass

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: allan, Assigned: allan)

References

()

Details

(Keywords: css3, qawanted)

Attachments

(6 files, 5 obsolete files)

We should support the :default pseudoclass defined in CSS3 UI:
http://www.w3.org/TR/css3-ui/#pseudo-default
and WebForms 2:
http://whatwg.org/specs/web-forms/current-work/#relation
Status: NEW → ASSIGNED
Attached patch Work in progress (obsolete) — Splinter Review
See bug 271720 comment 74.  Based on the specs, we should be supporting :default
on the appropriate options, checkboxes, and radios.
I have asked Ian about that in the past, he said it did not make sense and
therefore there is no relationship defined. See:
<http://listserver.dreamhost.com/pipermail/whatwg-whatwg.org/2005-March/003266.html>
The issue mentioned in comment 1 is different. It seems that Firefox always
choses the first button as default submit button, whether disabled or not and
irrelevant which form control field has focus. So Firefox is conforming to Web
Forms 2 in this regard. This also means we should pass this additional testcase:
 <http://annevankesteren.nl/test/css/ui/008>

(Only when :disabled is supported though.)
type DOM attribute change testcase:
 <http://annevankesteren.nl/test/css/ui/012>
Attached patch Patch (obsolete) — Splinter Review
(In reply to https://bugzilla.mozilla.org/show_bug.cgi?id=271720#c69)
> (From update of attachment 187107 [details] [diff] [review])
> >Index: content/html/content/src/nsGenericHTMLElement.cpp
> >@@ -3152,17 +3152,18 @@ nsGenericHTMLFormElement::SetForm(nsIDOM
> >+	// XXX (when) should SetForm() send notifications?
> >+	mForm->RemoveElement(this, PR_TRUE);
> 
> Please sort this out (look at the callers of SetForm and figure out what
> they're doing, etc).	Notifying unnecessarily would be a major perf issue (eg

> would force flushing out the content sink for no reason from the callsite at
>
<http://lxr.mozilla.org/seamonkey/source/content/html/document/src/nsHTMLConten

> tSink.cpp#842>; that same callsite could also leave to broken behavior on the

> part of the content sink).  It's entirely possible that SetForm() needs an
> aNotify argument of its own.

I've added an aNotify argument to SetForm() which is set to false by the
content sink. (AddElement() also got an aNotify argument)
 
> >Index: content/html/content/src/nsHTMLFormElement.cpp
> >+  NS_IMETHOD GetDefaultSubmitElement(nsIFormControl** aElement) const;
> 
> Again, I'd like this to be an NS_IMETHOD_(nsIFormControl*).  There's no
reason
> for the refcounting here.  I just checked with jst, and he agrees on this.

Done.
 
> @@ -1195,50 +1203,137 @@ nsHTMLFormElement::AddElement(nsIFormCon
> 
> >+	    mozAutoDocUpdate(document, UPDATE_CONTENT_STATE, PR_TRUE);
> 
> Ouch.  This will notify in the middle of sink notifications.	It'll also
notify
> in the middle of attr changes that have aNotify set to false.  Please fix
this
> to only notify when it should, ok?
> 
> Note that notifying in the middle of sink notifications may well be ok here
--
> we at least want to notify that oldElement is not the default anymore, since
it
> might have a frame.  But I really don't think we want to be doing this if
> aNotify is false on an attr change...

I do not know enough on how content sink, frames, etc, actually work to say in
which situations we need to notify from the content sink call. But
SetAttr(aNotify == PR_FALSE) was clearly a mistake.
 
> >+nsHTMLFormElement::ResetDefaultSubmitElement(PRBool aNotify)
> 
> >+	  if (type == NS_FORM_INPUT_SUBMIT ||
> >+	      type == NS_FORM_BUTTON_SUBMIT ||
> >+	      type == NS_FORM_INPUT_IMAGE) {
> 
> Maybe we should have an nsIFormControl function that does this?  followup
bug,
> perhaps?

nsIFormControl::IsSubmitControl(). (Should be const, but then GetType() should
be const too, which is part of bug 84400... so I'll let the consts surf in on
that one :-) )
 
> >+  // Inform about change
> 
> Even if aNotify is false?  That can't be right...

Most of the notify errors grounded in my misunderstanding of what actually
happens on ContentStatesChanged... I think I've gotten it now.
 
> >+  if (oldDefault || mDefaultSubmitElement) {
> >+	nsIDocument* document = GetCurrentDoc();
> >+	if (document) {
> 
> I really wish we could avoid notifying here for cases when the default
control
> is being removed from the document altogether (since in that case the fact
that
> its state changed really doesn't matter)...  But I bet we get into here
before
> oldDefault's document is nulled out in that case, right?

That's my understanding, yes.


> I'm not seeing any provisions for toggling the :default state for
> radio/checkbox when the relevant attr changes...
> 
> I don't see any provisions for toggling the :default state when the
"selected"
> attr changes.
 
My understanding is that :default for these should match the default selection
on load, so it shouldn't matter if attributes are changed. Or?

Only (known) bug left in this, is the case with multiple radio buttons having
checked="checked".
Attachment #190551 - Attachment is obsolete: true
Attachment #190695 - Flags: review?(bzbarsky)
(In reply to comment #4)
> he said it did not make sense

The email in which he said so is wrong on one major point -- it _is_ possible to
tell from the DOM which elements are the _default_ selected ones (the ones the
form would reset to; this is not quite the same as "originally selected", of
course).

(In reply to comment #7)
> I do not know enough on how content sink, frames, etc, actually work to say in
> which situations we need to notify from the content sink call.

You never do, basically.

> My understanding is that :default for these should match the default selection
> on load, so it shouldn't matter if attributes are changed. Or?

I would assume :default should match the default selection that would be reset
to if the reset button is pushed...  That makes a lot more sense than the "on
load" thing, especially if the whole form is constructed via the DOM (say I
create an input, insert in the DOM, then set the checked attribute; with your
current impl it won't match :default, but if I set the attr before inserting in
the DOM it will....  that seems wrong).

> Only (known) bug left in this, is the case with multiple radio buttons having
> checked="checked".

That's not really a bug; that's a case where behavior is so badly undefined that
just about anything we do is right.
I talked to Ian today. CSS3 UI should be followed. Web Forms 2 will hopefully be
updated this week. It will _probably_ say that :default should match the
defaultChecked DOM attribute for radio buttons and checkboxes and the
defaultSelected DOM attribute for option elements. (This means that although the
last radio button with defaultChecked set to true remains checked, but that more
can match :default.)
Attached file Testcase (obsolete) —
Attachment #190695 - Attachment is obsolete: true
Attachment #190695 - Flags: review?(bzbarsky)
Attached patch Patch v2 (obsolete) — Splinter Review
This one also handles dynamic changes to checked and selected attributes.
Attachment #190830 - Flags: review?(bzbarsky)
Attached file Testcase v2
Added buttons to toggle checked and selected.
Attachment #190819 - Attachment is obsolete: true
No longer blocks: 271720
It doesn't sound like this issue can help with bug 120527.

A button's dark border is supposed to show only when the Enter key will activate
it -- in otherwords not when another button or a textarea has focus.

Perhaps for bug 120527 we need a :-moz-default pseudo class to implement that?
Or perhaps we should use Neil's JS hack over there.
(In reply to comment #13)
> It doesn't sound like this issue can help with bug 120527.
> 
> A button's dark border is supposed to show only when the Enter key will activate
> it -- in otherwords not when another button or a textarea has focus.

You mean input I guess.

That's also what WF2 says ... CSS3UI is sort of vague. So :default should
possibly behave like that?
Yes. That is the behavior that would make sense.
(In reply to comment #14)
> (In reply to comment #13)
> > It doesn't sound like this issue can help with bug 120527.
> > 
> > A button's dark border is supposed to show only when the Enter key will activate
> > it -- in otherwords not when another button or a textarea has focus.
> 
> You mean input I guess.

Oh man, I constantly misread people these days :(... you definately mean textarea.
The default button doesn't depend on which input is currently focused, though. 
So I'm not sure it makes sense to affect matching of :default based on that.
(In reply to comment #17)
> The default button doesn't depend on which input is currently focused, though. 
> So I'm not sure it makes sense to affect matching of :default based on that.

Not if the inputs are in the same form, no, but for multiple forms it does
matter.  If :default only matches a submit element when it will actually be
triggered when the user presses enter, it would need to be controlled by focus
and blur on inputs and submit elements.
> If :default only matches a submit element when it will actually be
> triggered when the user presses enter

I'm not sure that makes a lot of sense as a definition of :default...
Blocks: 271720
No longer blocks: 271720
Allan, is that patch still current as far as needing review?
(In reply to comment #20)
> Allan, is that patch still current as far as needing review?

/me digs around his memory... finding it full of holes as usual...

Yes, I think it's there functionality-wise, but it needs to be brought up to
tip. I'll do that Monday, ok?
Sounds good.
Attachment #190830 - Flags: review?(bzbarsky)
Attachment #190830 - Attachment is obsolete: true
Attachment #199056 - Flags: review?(bzbarsky)
Blocks: 271720
No longer blocks: 271720
Blocks: selectors3
No longer blocks: selectors3
Comment on attachment 199056 [details] [diff] [review]
Patch v2 updated to tip

I'm sorry it took me so long to get to this....

>Index: content/events/public/nsIEventStateManager.h
>+#define NS_EVENT_STATE_DEFAULT      0x00008000 // CSS3-UI

This needs to be updated to tip, of course.

>Index: layout/style/nsCSSStyleSheet.cpp
>+    else if (nsCSSPseudoClasses::defaultPseudo == pseudoClass->mAtom) {
>+      result = STATE_CHECK(NS_EVENT_STATE_DEFAULT);
>+    }

This also....

>Index: content/html/content/public/nsIForm.h
>+   * @param aNotify send notifications?

  @param aNotify If true, send nsIDocumentObserver notifications as needed.

Same for other spots where this comment appears (there are 3-5 places in this patch).

>+   * Get the default submit element. If there's no default submit element, it
>+   * returns nsnull.

s/it returns null/return null/

>+   * Resets the default submit element. Used to find the new default submit

s/resets/reset/

>Index: content/html/content/public/nsIFormControl.h
>   NS_IMETHOD SetForm(nsIDOMHTMLFormElement* aForm,
>-                     PRBool aRemoveFromForm = PR_TRUE) = 0;
>+                     PRBool aRemoveFromForm = PR_TRUE,
>+                     PRBool aNotify = PR_TRUE) = 0;

Please make the aNotify arg non-default.  Better yet, make both non-default, since I doubt you can make aNotify non-default without making aRemoveFromForm so in a safe way.  That will make it clear which places are flushing when they should not, etc.

I suspect there are few enough callers that this shouldn't be a big deal.

>+  /**
>+   * Is the control a submit control?

What does that mean?  Perhaps this should give examples of the sort of thing that is considered a "submit control"?  Or say something like "a control which submits the form when activated by the user" or something?

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

>@@ -3261,17 +3263,17 @@ nsGenericHTMLFormElement::SetAttr(PRInt3

This function doesn't exist anymore.  The code is in nsGenericHTMLFormElement::AfterSetAttr now.  Merge that?

> nsGenericHTMLFormElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,

Which means your changes to this function are not needed, I believe.

>@@ -3365,16 +3376,30 @@ nsGenericHTMLFormElement::IntrinsicState
>+nsGenericHTMLFormElement::IsSubmitControl()
>+  if (type == NS_FORM_INPUT_SUBMIT ||
>+      type == NS_FORM_BUTTON_SUBMIT ||
>+      type == NS_FORM_INPUT_IMAGE)
>+  {
>+    return PR_TRUE;

How about:

  return type == NS_FORM_INPUT_SUBMIT ||
         type == NS_FORM_BUTTON_SUBMIT ||
         type == NS_FORM_INPUT_IMAGE;

instead?

>Index: content/html/content/src/nsHTMLButtonElement.cpp

>+nsHTMLButtonElement::IntrinsicState() const

Why not put this logic in nsGenericHTMLFormElement::IntrinsicState (conditioned on IsSubmitcontrol()) instead of putting it in multiple places?  I think I'd much prefer that.

>Index: content/html/content/src/nsHTMLFormElement.cpp

>@@ -1212,50 +1220,128 @@ nsHTMLFormElement::AddElement(nsIFormCon

>+    // First submit element added to the form
>+    if (!mDefaultSubmitElement) {
>+      mDefaultSubmitElement = aChild; // weak

Why no need to notify here?  Doesn't this change the child's state?  Or are we relying on all codepaths that lead to AddElement recreating frames (which I _believe_ is true)?  But then why notify in the other branch?  Or is the notification in that branch only needed for the old default?  But then why notify for both?

I guess my point is the two branches in this function should be consistent.

>+    else if (aNotify && !lastElement &&
>+             CompareFormControlPosition(aChild, mDefaultSubmitElement) < 0) {

So if aNotify is false and we're inserting somewhere in the middle of the form we can keep a stale mDefaultSubmitElement?  Why?  It seems to me that we should set mDefaultSubmitElement correctly no matter what aNotify is.  This should have been caught in testing, no?

>+nsHTMLFormElement::ResetDefaultSubmitElement(PRBool aNotify)
>+  nsCOMPtr<nsISimpleEnumerator> formControls;

I really wish we had a faster "internal" way of doing it.  This is ok for now, but want to file a followup bug on improving both this and the code in nsHTMLInputElement, perhaps?

>Index: content/html/content/src/nsHTMLInputElement.cpp

>@@ -502,27 +502,39 @@ nsHTMLInputElement::AfterSetAttr(PRInt32
>+  if (aName == nsHTMLAtoms::checked) {
>+    if (aNotify && !GET_BOOLBIT(mBitField, BF_PARSER_CREATING)) {

Why the mBitField check?  What's that for?  I think you should just use aNotify here and not second-guess the caller...

>+      if (aNotify) {

And this check is not needed.

Note that this function might need merging on trunk too.  :(

> nsHTMLInputElement::MaybeSubmitForm(nsPresContext* aPresContext)

Please post a -w version of the next patch in addition to the normal diff?  That would make this part much much easier to review.  Probably save me an hour or so, and I doubt it takes more than a minute or two to do.

>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    if (numTextControlsFound == 1) {

Why do you need that NS_ENSURE_SUCESS(rv, rv) there?

> nsHTMLInputElement::IntrinsicState() const
>+  } else if (mType == NS_FORM_INPUT_SUBMIT || mType == NS_FORM_INPUT_IMAGE) {

This part of the change can largely be undone if the :default handling for submits moves to nsGenericHTMLFormElement

I'd like to see a version of this patch with the comments addressed, but please let me know if you don't have time to do that and I'll see what I can do.  Again, sorry for the ludicrous review delay; next round will be much much faster.

Oh, and please attach the tests for this to the bug and make sure to check them into the regression tests when you land this?
Attachment #199056 - Flags: review?(bzbarsky) → review-
Martijn, do you think you might have time to get this updated and drive it in?
If it would be helpful I could probably find some time to merge this and run the test cases.
If you could, and fix up the review issues, that would be wonderful!
Okay, will do. I've got it merged and most of the review comments addressed, but it's failing some of the dynamic test cases. I'll try and figure those out.
Okay, I've got it working again and passing Allan's tests(Testcase_v2). The issue with dynamic modification was that the change to IsStateSelector got lost along the way. I'll attach a patch in a moment that addresses Boris's review comments. I also fixed several places where MozAutoDocUpdate was used without giving the instance a name. Responses to select review comments, the rest I think I fixed.

> > nsGenericHTMLFormElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
> 
> Which means your changes to this function are not needed, I believe.

I think they are still needed for the case of the type attribute being removed. Or did I miss something?

> >Index: content/html/content/src/nsHTMLButtonElement.cpp
> 
> >+nsHTMLButtonElement::IntrinsicState() const
> 
> Why not put this logic in nsGenericHTMLFormElement::IntrinsicState (conditioned
> on IsSubmitcontrol()) instead of putting it in multiple places?  I think I'd
> much prefer that.

Moved the part checking which element was the default submit element. Did you want more than that moved?

> >Index: content/html/content/src/nsHTMLFormElement.cpp
> 
> >@@ -1212,50 +1220,128 @@ nsHTMLFormElement::AddElement(nsIFormCon
> 
> >+    // First submit element added to the form
> >+    if (!mDefaultSubmitElement) {
> >+      mDefaultSubmitElement = aChild; // weak
> 
> Why no need to notify here?  Doesn't this change the child's state?  Or are we
> relying on all codepaths that lead to AddElement recreating frames (which I
> _believe_ is true)?  But then why notify in the other branch?  Or is the
> notification in that branch only needed for the old default?  But then why
> notify for both?
> 
> I guess my point is the two branches in this function should be consistent.

The new element only needs to be notified on in the case where the type attribute was added, which causes the form element to be removed and added. I'm trying to think if there's a case where that could cause the first branch to be hit.


> >+    else if (aNotify && !lastElement &&
> >+             CompareFormControlPosition(aChild, mDefaultSubmitElement) < 0) {
> 
> So if aNotify is false and we're inserting somewhere in the middle of the form
> we can keep a stale mDefaultSubmitElement?  Why?  It seems to me that we should
> set mDefaultSubmitElement correctly no matter what aNotify is.  This should
> have been caught in testing, no?

I changed this to notify, I think it has to.

> >+nsHTMLFormElement::ResetDefaultSubmitElement(PRBool aNotify)
> >+  nsCOMPtr<nsISimpleEnumerator> formControls;
> 
> I really wish we had a faster "internal" way of doing it.  This is ok for now,
> but want to file a followup bug on improving both this and the code in
> nsHTMLInputElement, perhaps?

Filed bug 347165

> 
> >Index: content/html/content/src/nsHTMLInputElement.cpp
> 
> >@@ -502,27 +502,39 @@ nsHTMLInputElement::AfterSetAttr(PRInt32
> >+  if (aName == nsHTMLAtoms::checked) {
> >+    if (aNotify && !GET_BOOLBIT(mBitField, BF_PARSER_CREATING)) {
> 
> Why the mBitField check?  What's that for?  I think you should just use aNotify
> here and not second-guess the caller...

This looks like it was copied from an earlier part of this function, I removed it.

Attached patch patch_v3(-w Not for checkin) (obsolete) — Splinter Review
Attachment #231940 - Flags: review?(bzbarsky)
Blocks: 347165
Comment on attachment 231940 [details] [diff] [review]
patch_v3(-w Not for checkin)

>+++ content/html/content/public/nsIForm.h

So... I'm not sure how you created this diff...

> // IID for the nsIFormManager interface
> #define NS_IFORM_IID    \
>-{ 0x6982e217, 0x8b5b, 0x4fd2,  \
>-  { 0x98, 0xc, 0x3a, 0xce, 0xc5, 0xe, 0x82, 0x88 } }

But that's not an IID this interface ever had.

>-class nsIForm : public nsIDOMNSHTMLFormElement
>+class nsIForm : public nsISupports

And this part makes no sense.  I'd like to see an updated diff of this file that makes sense...

>+++ content/html/content/public/nsIFormControl.h

>   virtual PRBool AllowDrop() = 0;
>+
>+
>+  /**

Remove one of those newlines.

>+++ content/html/content/src/nsGenericHTMLElement.cpp

> nsGenericHTMLFormElement::~nsGenericHTMLFormElement()
> {
>   // Clean up.  Set the form to nsnull so it knows we went away.
>-  SetForm(nsnull);
>+  SetForm(nsnull, PR_TRUE, PR_TRUE);
> }

So... won't this possibly dispatch notifications on this content that's being destroyed?  We don't want that.  I think it's safe to pass PR_FALSE for aNotify here (and perhaps assert that we are not the default content of our form or something?).

> nsGenericHTMLFormElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,

Why are the changes to this function needed?  Doesn't the removal in BeforeSetAttr and readdition in AfterSetAttr do what we want?  If this code goes away, we can also stop exposing ResetDefaultSubmitElement and make it a protected method of nsHTMLFormElement, right?

>@@ -3325,16 +3343,21 @@ nsGenericHTMLFormElement::IntrinsicState

>+  if (IsSubmitControl() && mForm && mForm->GetDefaultSubmitElement() == this) {

Why bother with the IsSubmitControl() check?  If we're the default submit element, we're a submit control.  Add an assert to that effect here if you want.

>+++ content/html/content/src/nsHTMLFormElement.cpp

>+nsHTMLFormElement::AddElement(nsIFormControl* aChild,
>+                              PRBool aNotify)
>+    // When the type of the form element changes the element is removed
>+    // and added. In that case we need to notify on both the old and new
>+    // elements. Otherwise, the new element will already have been
>+    // notified on.

I'm not following this comment...

>+    else if (!lastElement &&
>+             CompareFormControlPosition(aChild, mDefaultSubmitElement) < 0) {

Why aren't we checking aNotify?  We should be.

Note that the changes here don't actually address my comment on the version 2 patch....

What I _think_ is the right code flow is that we never notify on aChild, and notify on the current mDefaultSubmitElement if aChild is replacing it and aNotify is true.  But that assumes that all callers of AddElement lead to a frame reconstruct on aChild.  Is that true?

>+      if (document) {
>+        mozAutoDocUpdate upd(document, UPDATE_CONTENT_STATE, PR_TRUE);

Please use 

  MOZ_AUTO_DOC_UPDATE(document, UPDATE_CONTENT_STATE, PR_TRUE);

That will make sure people can't accidentally edit away the 'upd' part.  ;)

>+        mDefaultSubmitElement = aChild; // weak

Why is leaving an incorrect mDefaultSubmitElement OK when we have no document?  That seems wrong (and it should be easy to write tests to demonstrate that this leads to buggy behavior, no? we should add those tests to this bug so we make sure that all the relevant tests for this land in the regression tests).

>+nsHTMLFormElement::ResetDefaultSubmitElement(PRBool aNotify)

>+  if (aNotify && (oldDefault || mDefaultSubmitElement)) {

Can we assert here that oldDefault != mDefaultSubmitElement?  If not, should we check for that?

>+      mozAutoDocUpdate upd(document, UPDATE_CONTENT_STATE, PR_TRUE);

MOZ_AUTO_DOC_UPDATE

>+++ content/html/content/src/nsHTMLInputElement.cpp

>+          mozAutoDocUpdate upd(document, UPDATE_CONTENT_STATE, aNotify);

MOZ_AUTO_DOC_UPDATE

I'd like to see a patch without -w at some point too, btw, once things stabilize.

Do we relly want to notify if the "checked" attribute is changed on a control of types other than checkbox and radio?  I'd think not...

> nsHTMLInputElement::IntrinsicState() const

>+  } else if (mType == NS_FORM_INPUT_SUBMIT || mType == NS_FORM_INPUT_IMAGE) {

Why do you need this check?  You only do something interesting for NS_FORM_INPUT_IMAGE, so just check for that.

Looking at this function, I think we have interesting style resolution bugs when the type attribute of an HTML input changes; same thing for <button> elements.  Could you please file a followup bug on investigating that?  I think we want to fix them for 1.9...

Thanks for bringing this up to date!  Looking forward to the patch with the nits picked.
Attachment #231940 - Flags: review?(bzbarsky) → review-
Er, just realized I should respond to comment 29....

> I think they are still needed for the case of the type attribute being removed.

UnsetAttr calls BeforeSetAttr and AfterSetAttr, no?  And those handle the type attr being removed by removing and reinserting the element in the form?

> Moved the part checking which element was the default submit element. 

No, that was it.  Thanks!

> The new element only needs to be notified on in the case where the type
> attribute was added

I think we'll have to generally handle type attribute changes in that followup bug I was talking about in my review comment.

> I'm trying to think if there's a case where that could cause the first branch 
> to be hit.

Parsing hits it, no?

> I changed this to notify, I think it has to.

Like I said, I don't buy this change.  We should talk about this; maybe catch me on irc?

Again, thank you!
Attachment #231940 - Attachment is obsolete: true
Attachment #233713 - Flags: review?(bzbarsky)
Attached patch patch_v4Splinter Review
Not -w.

I also fixed two random whitespace/newline/parenthesis changes that crept in:

+    if (aNotify &&
+        (aName == nsHTMLAtoms::disabled && CanBeDisabled())) {

and 

 // nsHTMLFormElement
-
> Looking at this function, I think we have interesting style resolution bugs
> when the type attribute of an HTML input changes; same thing for <button>
> elements.  Could you please file a followup bug on investigating that?  I think
> we want to fix them for 1.9...

I tried a few test cases and couldn't find any bugs. bz, could you point me in the right direction?
Something like this should do it:

CSS:

span { color: green }
:default + span { color: red }

HTML:

<input type="checkbox" checked="checked">

JS:  Remove the type attribute from the input of a setTimeout.
Er, except use :checked instead of :default -- The :default code actually handles this case... ;)
Comment on attachment 233713 [details] [diff] [review]
patch_v4(-w Not for check in)

This looks great!  Thanks a ton for fixing this up!

r+sr=bzbarsky
Attachment #233713 - Flags: superreview+
Attachment #233713 - Flags: review?(bzbarsky)
Attachment #233713 - Flags: review+
Attached patch bustage_fixSplinter Review
This might fix it?
Checked in that patch, plus a bustage fix.  jpl24 says he'll create some fully automated testcases that I could add to the regression tests.  Filed bug 348809 on the type-change issues.
Attached file default_testacases
Test cases for the default pseudoclass. 

bz, these tests start both in numbering and in test coverage from the ones you wrote. I included a read me file that describes what the tests cover.
Note that dynamic test cases numbers 10 and 12 currently fail. I think this test case behavior is correct and caused by content state changed notification on the removed element coming too late, after the element is removed from the document. Follow up bug?
> Note that dynamic test cases numbers 10 and 12 currently fail.

Yeah, and with the patch for bug 348809 a few of the "add" testcases will fail too, because I removed a notification that really should not be needed.  The problem you're seeing here is bug 229915.

I checked in those testcases to the regression tests.

This bug is fixed.  Thank you!
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Depends on: 349355
Flags: in-testsuite?
Comment on attachment 234622 [details]
default_testacases

I added the test cases in here to mochitest. There were two failing, so they are running as todo()

RCS file: /cvsroot/mozilla/testing/mochitest/tests/test_bug302186.html,v
done
Checking in tests/test_bug302186.html;
/cvsroot/mozilla/testing/mochitest/tests/test_bug302186.html,v  <--  test_bug302186.html
initial revision: 1.1
done
Flags: in-testsuite? → in-testsuite+
Depends on: 371061
I just realized that the tests that jpl24 started from (the static/dynamicDefault tests at http://web.mit.edu/bzbarsky/www/testcases/css-selectors/) never made it into the tree.  I went ahead and landed them.
You need to log in before you can comment on or make changes to this bug.