Toolkit uses toolbar[mode="icons"] instead of *[buttonstyle="pictures"] and similar changes are needed for text-only toolbars.

Obviously I've only tried this out on Windows...

Comment on attachment 292618 [details] [diff] [review] Proposed patch Index: suite/themes/modern/jar.mn This change is part of a different bug, right? Also, why are you removing the #nav-bar-buttons bits?

(In reply to comment #2) > Index: suite/themes/modern/jar.mn > This change is part of a different bug, right? Bug 240393 > Also, why are you removing the #nav-bar-buttons bits? Bug 407744 removed the #nav-bar-buttons

(In reply to comment #3) > (In reply to comment #2) > > Also, why are you removing the #nav-bar-buttons bits? > Bug 407744 removed the #nav-bar-buttons That change was supposed to be part of this patch. It's needed for bug 394288. I must have edited that out instead of the jar.mn change by mistake.

Oops I confused toolbar_button_box with nav-bar-buttons

OK, this version should have all the missing bits.

> +toolbar[mode="text"] > .toolbarbutton-1 > .... [18:13:10] <stefanh> NeilAway: the child selectors [18:36:59] <NeilAway> stefanh: also the worst that can happen is that during customisation you see both icons and text Neil, I think that this is unacceptable. For example if the toolbar is currently in smallicon+icononly mode and you enter customization, the toolbar buttons will become large, acquire text labels and the padding and margins will change. I find this rather unpleasant. In Minefield, the iconsize/mode of the toolbar buttons do not change when customization is entered. I hope that you will reconsider and make the first child selector a descendant selector instead e.g.: toolbar[mode="text"] .toolbarbutton-1 > .... I thought of adding additional rules such as: toolbar[mode="text"] > * > .toolbarbutton-1 > .... But decided that this would be too unwieldy. In addition it is theoretically possible that an extension could do something like: <toolbaritem> <toolbarbutton .../> <toolbarbutton .../> </toolbaritem> So descendant selectors please? I shall be in your debt forever (or until next Thursday, whichever comes first).

Comment on attachment 292958 [details] [diff] [review] Updated patch I can't make this work on mac, no matter how I try I can't change the pref.

(In reply to comment #8) > (From update of attachment 292958 [details] [diff] [review]) > I can't make this work on mac, no matter how I try I can't change the pref. > Uh, forget about it (old tree + missed a few bits from the patch)

This version uses descendent selectors so it should be a bit smaller.

(In reply to comment #10) > This version uses descendent selectors so it should be a bit smaller. Thanks for that Neil. > + chromeclass="chromeclass-toolbar-additional" Sorry for sounding dense, but how does chromeclass="foo" work? I've looked for css rules that use this, and for inherit directives in xbl bindings and failed to find anything relevant. In fact I can't find anything in LXR that uses |chromeclass=| .

Comment on attachment 293331 [details] [diff] [review] Alternative version It occurs to me that switching to descendant selectors deserves a new superreview.

(In reply to comment #11) >Sorry for sounding dense, but how does chromeclass="foo" work? http://mxr.mozilla.org/seamonkey/source/toolkit/content/xul.css#40

jag pointed out what Ratty meant...

Comment on attachment 293384 [details] [diff] [review] Fixed version Looks good to me and not spotted in problems in use over the last few days.

Fix checked in.