Closed Bug 128585 Opened 19 years ago Closed 13 years ago

Support CSS :first-of-type, :only-of-type, and :last-of-type pseudo-class selectors

Categories

(Core :: CSS Parsing and Computation, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
mozilla1.9.1a1

People

(Reporter: imz, Assigned: dbaron)

References

Details

(4 keywords)

Attachments

(2 files, 1 obsolete file)

Mozilla 0.9.8 doesn't accept ":last-of-type" selector as specified in
http://www.w3.org/TR/2001/CR-css3-selectors-20011113/.

When viewing an XHTML 1.1 page with a stylesheet that uses a selector like:

li:last-of-type

one doesn't see the effect of this style rule. (An example of such a page you
can find at the URL linked with this report.)

Note, that a similar ":last-child" pseudoclass selector seems to be processed by
Mozilla the right way.
The same as at the URL given in the bug report.
There is also an official test by W3C (on which Mozilla fails) for the same thing:
http://www.w3.org/Style/CSS/Test/CSS3/Selectors/20020115/xhtml/tests/css3-modsel-35.xml
Keywords: css3, testcase
-> enhancement; glazman (although this probably requires adding some interesting
lazily constructed data structures to the RuleProcessorData).
Assignee: dbaron → glazman
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reconfirmed using FizzillaCFM/2002071508. Setting Platform=All.
Hardware: PC → All
Blocks: selectors3
Summary: CSS3: ":last-of-type" pseudoclass selectors are not processed → Support :last-of-type pseudo-class selector
Alias: css-last-of-type
QA Contact: ian → style-system
Is there a bug for :first-of-type? or is it covered within some other bug? I can't seem to find any specific reference to it.
Duplicate of this bug: 419282
This bug should cover all three; there's no reason we'd implement one without the others.
Summary: Support :last-of-type pseudo-class selector → Support CSS :first-of-type, :only-of-type, and :last-of-type pseudo-class selectors
Alias: css-last-of-type
Duplicate of this bug: 419278
Duplicate of this bug: 419283
Flags: blocking1.9?
Assignee: daniel → dbaron
Not a release blocker.
Flags: blocking1.9? → blocking1.9-
Duplicate of this bug: 419264
Attachment #308232 - Flags: superreview?(bzbarsky)
Attachment #308232 - Flags: review?(bzbarsky)
Comment on attachment 308232 [details] [diff] [review]
patch

>+++ b/layout/style/nsCSSRuleProcessor.cpp
>+    else if (nsCSSPseudoClasses::firstOfType == pseudoClass->mAtom ||
>+             nsCSSPseudoClasses::lastOfType == pseudoClass->mAtom ||
>+             nsCSSPseudoClasses::onlyOfType == pseudoClass->mAtom) {

Would it make sense to also combine firstChild/lastChild/onlyChild like this as well, now that we have this index-caching thing in place?  Followup bug is fine.

r+sr=bzbarsky
Attachment #308232 - Flags: superreview?(bzbarsky)
Attachment #308232 - Flags: superreview+
Attachment #308232 - Flags: review?(bzbarsky)
Attachment #308232 - Flags: review+
(In reply to comment #13)
> Would it make sense to also combine firstChild/lastChild/onlyChild like this as
> well, now that we have this index-caching thing in place?  Followup bug is
> fine.

I don't see why -- those are computationally much less complex (except for bizarre edge cases with ridiculous amounts of text or comments -- though maybe that's not quite as bizarre as I think) since the looping over children can stop at the first/last child element.
Attached patch patchSplinter Review
Merged on top of the revised patch in bug 75375.
Attachment #308232 - Attachment is obsolete: true
With the changes in this patch, GetNthIndex will also stop at the first/last child element if we're doing the aCheckEdgeOnly case.  So I think the computational complexity for a single selector occurrence is about the same.  The difference is that GetNthIndex will cache, so if the selector occurs more than once we win.

And yeah, having a lot of text is not that bizarre, really.
http://hg.mozilla.org/mozilla-central/index.cgi/rev/84d1f9d39ac3

-> fixed (for the release after Firefox 3.0.*)
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
Status: RESOLVED → VERIFIED
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Sorry, I hit the wrong button. not verified yet.
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
marking wanted1.9.1? to get this in the triage queue.  P2.  Please change if you disagree.
Flags: wanted1.9.1?
Priority: -- → P2
Target Milestone: mozilla2.0 → mozilla1.9.1
Keywords: dev-doc-needed
(In reply to comment #13)
> (From update of attachment 308232 [details] [diff] [review])
> >+++ b/layout/style/nsCSSRuleProcessor.cpp
> >+    else if (nsCSSPseudoClasses::firstOfType == pseudoClass->mAtom ||
> >+             nsCSSPseudoClasses::lastOfType == pseudoClass->mAtom ||
> >+             nsCSSPseudoClasses::onlyOfType == pseudoClass->mAtom) {
> 
> Would it make sense to also combine firstChild/lastChild/onlyChild like this as
> well, now that we have this index-caching thing in place?  Followup bug is
> fine.
> 
> r+sr=bzbarsky
> 

:first-child and :last-child are already implemented in Firefox 2, I use them all the time.
Yes; the point of my comment was that we should make those use the new code as well, instead of the implementation they use right now.
Target Milestone: mozilla1.9.1 → mozilla1.9.1a1
Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008070902 Minefield/3.1a1pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008070903 Minefield/3.1a1pre ID:2008070903
Status: RESOLVED → VERIFIED
Flags: wanted1.9.1? → wanted1.9.1+
Keywords: fixed1.9.1
Keywords: verified1.9.1
Keywords: fixed1.9.1
Thanks! It works in Epiphany 2.26.3 (Mozilla/5.0 (X11; U; Linux i686; en; rv:1.9.1.4pre) Gecko/20080528 Epiphany/2.22 Firefox/3.5).

Should I somehow close this bugreport now?
VERIFIED FIXED means that the report has been closed with the resolution that code has been changed to fix it and that someone has subsequently verified that it has indeed been fixed.
You need to log in before you can comment on or make changes to this bug.