544834 - implement :-moz-any() for selector grouping (OR) between combinators

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P4)

Description

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

I have a patch to implement a :-moz-any() pseudo-class that allows selector grouping between combinators.  For example:
  ul > li, ol > li, dir > li, menu > li
could be combined into:
  :-moz-any(ul, ol, dir, menu) > li

I expect this will speed up some existing selectors in our user-agent stylesheets, and could be even more useful when implementing some things that are in HTML5.

Comment 1

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

Attachment: patch 1: cleanup pseudo-class argument parsing

This has better error handling for pseudo-class argument parsing; I couldn't think of any existing cases where it changes behavior, but patch 2 introduces some.

Comment 2

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

Attachment: patch 2: implement :-moz-any()

Comment 3

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

Attachment: patch 3: use :-moz-any() in html.css

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 425761 [details] [diff] [review]
patch 1: cleanup pseudo-class argument parsing

r=bzbarsky

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 425763 [details] [diff] [review]
patch 3: use :-moz-any() in html.css

r=bzbarsky; you can use this orthe xul|videocontrols styles too, right?

Comment 6

[Boris Zbarsky [:bzbarsky]]

And forms.css can use this stuff a bit for the .anonymous-div rules.

Do we want blizzard to blog this and dev-doc-needed it?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 425762 [details] [diff] [review]
patch 2: implement :-moz-any()

AddRule needs to look at :-moz-any pseudos, I think.  Without that there will be dynamic change bugs here where attr, class, id, state selectors inside :-moz-any will not respond properly to DOM changes.  r- due to that; please make that AddRule change and add tests for that code?

Having to add aStateMask to all those callback functions is kinda annoying.  I wonder whether we should just go with a switch() after all instead of an array of function pointers.  Certainly the more arguments the functions have the more attractive the switch() is performance-wise, and the switch() version is likely easier to maintain....

Maybe a followup bug (filed on me if you want) to make that change?

Comment 8

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

Attachment: patch 2: implement :-moz-any()

Here's a version of patch 2 that I believe addresses your previous comments.

It still doesn't fix specificity.  However, I think that's a decent amount of work, and my current inclination is to get the patch in (and maybe even blog about it) and get a bug filed on getting the right specificity (which probably means more dynamic specificity computation).

Comment 9

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

Attachment: patch 2: implement :-moz-any()

It turns out that patch stopped working at some point, I think due to zwol's selector parsing refactoring.  This fixes it by adding an extra check in ParseSelectorGroup.

Comment 10

[Boris Zbarsky [:bzbarsky]]

David, you don't have a sane interdiff of that last patch against what I reviewed before by chance?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 437152 [details] [diff] [review]
patch 2: implement :-moz-any()

> +  nsTArray<nsCSSSelector*>* stateArray = &aCascade->mStateSelectors;
> +  nsTArray<nsCSSSelector*>* classArray = &aCascade->mClassSelectors;
> +  nsTArray<nsCSSSelector*>* idArray = &aCascade->mIDSelectors;

Those locals are used once each, so not sure they're really needed.

r=bzbarsky with that fixed.

Comment 12

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

I filed bug 561154 for doing the right thing about specificity.

I have the patches ready to push in my queue; doesn't seem like there's much hope of actually pushing them this week.

Comment 13

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

http://hg.mozilla.org/mozilla-central/rev/008d2f6c9d27
http://hg.mozilla.org/mozilla-central/rev/6efa2358f849
http://hg.mozilla.org/mozilla-central/rev/8e552e1afa49

Comment 14

[Dão Gottwald [:dao]]

(In reply to comment #0)
> I have a patch to implement a :-moz-any() pseudo-class that allows selector
> grouping between combinators.  For example:
>   ul > li, ol > li, dir > li, menu > li
> could be combined into:
>   :-moz-any(ul, ol, dir, menu) > li

Would a > :-moz-any(x,y) perform better or worse than a > x, a > y?

Comment 15

[[e-mail]]

Comment on attachment 425763 [details] [diff] [review]
patch 3: use :-moz-any() in html.css

> /* 2 deep unordered lists use a circle */
>-ol ul,   ul ul,   menu ul,   dir ul,
>-ol menu, ul menu, menu menu, dir menu,
>-ol dir,  ul dir,  menu dir,  dir dir {
>+:-moz-any(ol, ul, menu, dir) ul,
>+:-moz-any(ol, ul, menu, dir) menu,
>+:-moz-any(ol, ul, menu, dir) dir {
>   list-style-type: circle;
> }
Dao, does this answer your question?

Comment 16

[Dão Gottwald [:dao]]

I understand that this is better:

:-moz-any(x,y) > a

But I'm asking about:

a > :-moz-any(x,y)

Comment 17

[Boris Zbarsky [:bzbarsky]]

> Would a > :-moz-any(x,y) perform better or worse than a > x, a > y?

Almost certainly worse.  See >https://developer.mozilla.org/en/Writing_Efficient_CSS>: the former is in bucket 4, while the latter two are in bucket 3.

Comment 18

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

Yes, worse, for now, at least (though it could probably be changed).  Right now we don't attempt to use :-moz-any() selectors when determining buckets for the RuleHash.  We could, but it would probably double the size of the implementation.

Comment 19

[d]

Should this be added to https://developer.mozilla.org/en/Upcoming_Firefox_features_for_developers?

Comment 20

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

(In reply to comment #19)
> Should this be added to
> https://developer.mozilla.org/en/Upcoming_Firefox_features_for_developers?

Yes, once developer.mozilla.org is working again.

Also more info at http://dbaron.org/log/20100424-any

Comment 21

[Boris Zbarsky [:bzbarsky]]

We should consider having a hacks post about this too.

Comment 22

[Paul Rouget [:paul]]

(In reply to comment #21)
> We should consider having a hacks post about this too.

Definitely. I was thinking about cross posting dbaron's post (with some more context).

David, is it ok for you?

(In reply to comment #20)
> (In reply to comment #19)
> > Should this be added to
> > https://developer.mozilla.org/en/Upcoming_Firefox_features_for_developers?

Done.

Comment 23

[Lars Gunther]

Please mention in all material that this will be extremely valuable in HTML5 when it comes to sectioning and headings.

Differentiating between headings based on how deep they are inside sectioning elements is the missing key for those elements to take off in a usable fashion.

h1
-moz-any(section, article, aside, nav) h1 = h2
-moz-any(section, article, etc) -moz-any(section, article, etc) h1 = h3

Without a selector like this one, the variations are endless (or 4 to the power of the depth minus 1).

Comment 24

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

(In reply to comment #22)
> (In reply to comment #21)
> > We should consider having a hacks post about this too.
> 
> Definitely. I was thinking about cross posting dbaron's post (with some more
> context).
> 
> David, is it ok for you?

Sure, it's ok with me.

Comment 25

[Eric Shepherd [:sheppy]]

Added document here; please review and let me know if there are any issues. It's not as clear as I like because my CSS skills are not über-l337.

https://developer.mozilla.org/en/CSS/:-moz-any