I came across this today: > #PanelUI-mainView toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active) { >} Bug 561154 describes the performance issues with using -moz-any as the right-most selector. In this scenario, it is equivalent to using the universal selector. We should just expand this rule so that it doesn't have the performance issues. Also see https://developer.mozilla.org/en-US/docs/CSS/:any#Issues_with_performance_and_specificity for some examples.

Comment on attachment 743433 [details] [diff] [review] Patch Review of attachment 743433 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for flagging me for review on this. I'll ask dbaron in-person tomorrow to confirm my comment below before finalizing the review. ::: browser/themes/shared/customizableui/panelUIOverlay.inc.css @@ -211,5 @@ > 0 0 2px hsla(210,54%,20%,.1); > } > > -#PanelUI-subViews toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active), > -#PanelUI-mainView toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active) { I'm pretty sure that it's okay to use :-moz-any() as _part_ of the right-most selector as long as there is a faster bucket that the right-most selector also falls into. In this case, the right-most selector is considered firstly a tag selector.

Comment on attachment 743433 [details] [diff] [review] Patch Review of attachment 743433 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for flagging me for review on this. I'll ask dbaron in-person tomorrow to confirm my comment below before finalizing the review. ::: browser/themes/shared/customizableui/panelUIOverlay.inc.css @@ +184,5 @@ > #customization-palette .toolbarbutton-text { > display: none; > } > > +:-moz-any(#PanelUI-mainView, #PanelUI-subViews) toolbarbutton { One thing to watch out for is that the specificity of :-moz-any() is incorrect. It uses the specificity of a pseudo-class rather than the specificity of its arguments. This could be a problem if there are also selector rules like `.foo toolbarbutton.bar` that also apply to these elements. See bug 561154. @@ -211,5 @@ > 0 0 2px hsla(210,54%,20%,.1); > } > > -#PanelUI-subViews toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active), > -#PanelUI-mainView toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active) { I'm pretty sure that it's okay to use :-moz-any() as _part_ of the right-most selector as long as there is a faster bucket that the right-most selector also falls into. In this case, the right-most selector is considered firstly a tag selector.

Ack, stupid splinter review, dammit!

Comment on attachment 743433 [details] [diff] [review] Patch Review of attachment 743433 [details] [diff] [review]: ----------------------------------------------------------------- r+ with comments addressed. ::: browser/themes/shared/customizableui/panelUIOverlay.inc.css @@ +184,5 @@ > #customization-palette .toolbarbutton-text { > display: none; > } > > +:-moz-any(#PanelUI-mainView, #PanelUI-subViews) toolbarbutton { Please make sure that these rules still are applied after the decreased specificity here. @@ -211,5 @@ > 0 0 2px hsla(210,54%,20%,.1); > } > > -#PanelUI-subViews toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active), > -#PanelUI-mainView toolbarbutton:not([disabled]):-moz-any([open],[checked],:hover:active) { I verified with David Baron that the removal of :-moz-any() is not necessary here.

Nits addressed: https://hg.mozilla.org/projects/jamun/rev/71d47560397f Thanks for the speedy review!

This broke the borders of the widgets in the customization panel. :-(

Comment on attachment 744547 [details] Screenshot I'm unable to reproduce this on latest Jamun using a fresh profile on Win7. Could there have been something wrong with your local build?

(In reply to Jared Wein [:jaws] from comment #8) > Comment on attachment 744547 [details] > Screenshot > > I'm unable to reproduce this on latest Jamun using a fresh profile on Win7. > Could there have been something wrong with your local build? His screenshot is of OS X, I think. Could it be that there are more specific selectors in other stylesheets on OS X?

(adding an attachment on a bug should put you in the CC list by default. Grrr, bugzilla. Sorry for the late response, hello from Paris!) (In reply to Frank Yan (:fryn) from comment #9) > (In reply to Jared Wein [:jaws] from comment #8) > > Comment on attachment 744547 [details] > > Screenshot > > > > I'm unable to reproduce this on latest Jamun using a fresh profile on Win7. > > Could there have been something wrong with your local build? > > His screenshot is of OS X, I think. Could it be that there are more specific > selectors in other stylesheets on OS X? This is indeed OS X, and I did check on a nightly with a fresh profile (and just did again, to be sure! :-) ).

I backed it out locally on OS X and it fixed the issue for me. Frank is on the right track here. Since switching to :-moz-any, the specificity of these rules was changed in a detrimental way. The patch didn't actually make any changes that needed to happen, and so it would be easiest if we just did a straight backout while we can. We should also merge this UX pretty promptly, as it is a highly visible regression for users of UX.

Comment on attachment 744982 [details] [diff] [review] Backout of patch Yeah, this works. Still, even if this doesn't fix anything, it was a good cleanup to attempt, I guess... so it might be worth doing it again but then adjusting something else for OS X so as to fix this issue? I would look myself but I'm on holiday until Sunday night. I may have time to take a peek tomorrow morning, but no promises. :-)

Well, it's a good clean up attempt, but until bug 561154 is fixed it's not really worth the time. The change in specificity is drastic enough that it will make future modifications to the CSS have unexpected and unintended consequences.

Thanks for the review! Backed out on Jamun, I'll close the bug now so it doesn't have to wait until the merge to UX. https://hg.mozilla.org/projects/jamun/rev/b21b5e5e9ad7