Closed Bug 544834 Opened 12 years ago Closed 12 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 2 obsolete files)

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.
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.
Attachment #425761 - Flags: review?(bzbarsky)
Attached patch patch 2: implement :-moz-any() (obsolete) — Splinter Review
Attachment #425762 - Flags: review?(bzbarsky)
Attachment #425763 - Flags: review?(bzbarsky)
Comment on attachment 425761 [details] [diff] [review]
patch 1: cleanup pseudo-class argument parsing

r=bzbarsky
Attachment #425761 - Flags: review?(bzbarsky) → review+
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?
Attachment #425763 - Flags: review?(bzbarsky) → review+
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 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?
Attachment #425762 - Flags: review?(bzbarsky) → review-
Attached patch patch 2: implement :-moz-any() (obsolete) — Splinter Review
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).
Attachment #425762 - Attachment is obsolete: true
Attachment #435004 - Flags: review?(bzbarsky)
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.
Attachment #435004 - Attachment is obsolete: true
Attachment #437152 - Flags: review?(bzbarsky)
Attachment #435004 - Flags: review?(bzbarsky)
David, you don't have a sane interdiff of that last patch against what I reviewed before by chance?
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.
Attachment #437152 - Flags: review?(bzbarsky) → review+
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.
Priority: -- → P4
Target Milestone: --- → mozilla1.9.3a5
(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 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?
I understand that this is better:

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

But I'm asking about:

a > :-moz-any(x,y)
Keywords: dev-doc-needed
> 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.
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.
(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
We should consider having a hacks post about this too.
(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.
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).
(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.
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
Duplicate of this bug: 62892
You need to log in before you can comment on or make changes to this bug.