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

Status: VERIFIED FIXED

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

Description

[Ivan Zakharyaschev]

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.

Comment 1

[Ivan Zakharyaschev]

Attachment: A sample XHTML1.1 page that demonstartes the bug.

The same as at the URL given in the bug report.

Comment 2

[Ivan Zakharyaschev]

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

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

-> enhancement; glazman (although this probably requires adding some interesting
lazily constructed data structures to the RuleProcessorData).

Comment 4

[Greg K.]

Reconfirmed using FizzillaCFM/2002071508. Setting Platform=All.

Comment 5

[Jose Fandos]

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.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug should cover all three; there's no reason we'd implement one without the others.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Applies on top of attachment 308228 [details] [diff] [review], attachment 308230 [details] [diff] [review], and attachment 308231 [details] [diff] [review], all from bug 75375.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Not a release blocker.

Comment 13

[Boris Zbarsky [: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

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Merged on top of the revised patch in bug 75375.

Comment 16

[Boris Zbarsky [:bzbarsky]]

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.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/index.cgi/rev/84d1f9d39ac3

-> fixed (for the release after Firefox 3.0.*)

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Sorry, I hit the wrong button. not verified yet.

Comment 19

[Damon Sicore (:damons)]

marking wanted1.9.1? to get this in the triage queue.  P2.  Please change if you disagree.

Comment 20

[Eric Shepherd [:sheppy]]

These are documented now:

http://developer.mozilla.org/en/docs/CSS::first-of-type
http://developer.mozilla.org/en/docs/CSS::last-of-type
http://developer.mozilla.org/en/docs/CSS::only-of-type

Comment 21

[Matthew Knowlton]

(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.

Comment 22

[Boris Zbarsky [:bzbarsky]]

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.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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

Comment 24

[Ivan Zakharyaschev]

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?

Comment 25

[Damian Shaw [Quan]]

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.