Closed Bug 237568 Opened 17 years ago Closed 16 years ago

Add CSS3 pseudo-class selector :only-child

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.8alpha1

People

(Reporter: svl-bmo, Assigned: dbaron)

References

Details

(Keywords: css3, Whiteboard: [patch])

Attachments

(1 file, 1 obsolete file)

1.59 KB, patch
svl-bmo
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
 
Attached patch patch (obsolete) — Splinter Review
Pretty self-evident patch, mostly based on :first-child and :last-child. Tested
with the :only-child tests (36, 81 and 81b) from the CSS3 test-suite, as well
as with some contraptions of my own.
Attachment #143980 - Flags: review?(bzbarsky)
Are there issues with XBL and a need to check that the child you've found is
actually the node you're matching?  I think the code might be simpler if you
tested equality, too.

There are also dynamic change bugs that this doesn't fix, including some with
incremental loading.

First child / last child information should also probably move into
SelectorMatchesData at some point (which would mean we ight want to implement
:only-child as :first-child && :last-child).
Comment on attachment 143980 [details] [diff] [review]
patch

>+        if (onlyChild) {

I think David is right.  You want to check here that data.mContent ==
onlyChild.  That allows you to not do the work of looking for moreChild in
cases when it's not.  r=bzbarsky with that change.

I think the dynamic issues shouldn't really block this... we have the same
issues with other pseudos (eg :empty, :last-child, etc) and we need to solve
them all together anyway (and soonish, imo).

I can see pushing this info up into SelectorMatchesData (lazily).  Is there a
bug on that?
Attachment #143980 - Flags: superreview?(dbaron)
Attachment #143980 - Flags: review?(bzbarsky)
Attachment #143980 - Flags: review+
Attached patch updated patchSplinter Review
Apologies for taking so long for a one-line change, but internet access has
unfortunately been scarce lately.

dbaron: Being unable to find any "SelectorMatchesData" in the code, I'm
assuming you mean the RuleProcessorData argument passed along to
SelectorMatches; moving information about an element being a first or last
child (and first or last node I assume) into that should be no problem, even
for me, but is that efficient? Or am I missing something? (What's the meaning
of "lazily" in this context?)
(If I'm being awfully obtuse, feel free to ignore me, I'm quite aware of how
far I still have to go before I really start to get a grip on the code involved
here.) :)

But if that _is_ the idea, I'll look into doing so next. (I'm perfectly okay
with the current approach being abandoned by the wayside, realy only created
this as a learning experience.)
Attachment #143980 - Attachment is obsolete: true
Attachment #143980 - Flags: superreview?(dbaron)
Comment on attachment 144888 [details] [diff] [review]
updated patch

carrying over r=
Attachment #144888 - Flags: superreview?(dbaron)
Attachment #144888 - Flags: review+
Attachment #144888 - Flags: superreview?(dbaron) → superreview+
sr=dbaron, although I'm not crazy about the idea of introducing new selectors
with dynamic change bugs.  bz, do you think I should try to get this in 1.7final
or wait for 1.8alpha?
Whiteboard: [patch]
I would say wait for 1.8a, if only because there is a non-negligible chance that
we'll solve the dynamic update problem in general in the 1.8 timeframe, which
would address your concern about adding buggy selectors.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha
Keywords: css3
Hardware: PC → All
This patch may be checked in now. Note that 'nsCSSPseudoClassList.h' differs a
bit from what is in the patch. ':first-node' and ':last-node' are now
':-moz-first-node' and ':-moz-last-node'.
Patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Using the tests cases mentioned in the topic start from the W3C... (2004052601)

-> VERIFIED FIXED
Status: RESOLVED → VERIFIED
Doesn't work for me in Firefox nightlies. Regression?
Which firefox nightlies?  Trunk, or branch?  If the latter, note checkin date on
this bug, please.
Sorry, branch nightly.

I see on the bug I filed it was posted that "That fix is only on the trunk, and
Firefox diverged from the trunk prior to the bug fix. You won't see it in
Firefox until it returns to the trunk in a post-1.0 milestone release."
You need to log in before you can comment on or make changes to this bug.