After this patch is landed, we can update the OS X styles to use this class. (or just do that in this patch)

Comment on attachment 814415 [details] [diff] [review] Patch Review of attachment 814415 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/windows/browser.css @@ +440,5 @@ > } > > #nav-bar .toolbarbutton-1:not(:hover):not(:active):not([open]) > .toolbarbutton-menubutton-dropmarker::before, > +#nav-bar .toolbaritem-with-separator > .toolbarbutton-1:not(:active):not([open]) + .toolbarbutton-1:not(:hover):not(:active):not([open])::before, > +#nav-bar .toolbaritem-with-separator > .toolbarbutton-1[disabled]:not(:active):not([open]) + .toolbarbutton-1[disabled]:not(:active):not([open])::before { Why the separate [disabled] rule?

Comment on attachment 814415 [details] [diff] [review] Patch Review of attachment 814415 [details] [diff] [review]: ----------------------------------------------------------------- r=me, but we should probably make sure that we deal with OS X and Linux as well.

Comment on attachment 814415 [details] [diff] [review] Patch "toolbaritem-with-separator" suggests that there will be one separator in total, but there can be multiple separators, the exact number being dependent on the number of buttons. Please come up with a better class name.

(In reply to :Gijs Kruitbosch from comment #2) > Comment on attachment 814415 [details] [diff] [review] > Patch > > Review of attachment 814415 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/themes/windows/browser.css > @@ +440,5 @@ > > } > > > > #nav-bar .toolbarbutton-1:not(:hover):not(:active):not([open]) > .toolbarbutton-menubutton-dropmarker::before, > > +#nav-bar .toolbaritem-with-separator > .toolbarbutton-1:not(:active):not([open]) + .toolbarbutton-1:not(:hover):not(:active):not([open])::before, > > +#nav-bar .toolbaritem-with-separator > .toolbarbutton-1[disabled]:not(:active):not([open]) + .toolbarbutton-1[disabled]:not(:active):not([open])::before { > > Why the separate [disabled] rule? This is because we still want to show the separator in the case that the buttons are disabled (since we aren't going to be showing the button outline in this case). I forgot to remove the :active from that rule too, will do it with the next patch. (In reply to Dão Gottwald [:dao] from comment #4) > Comment on attachment 814415 [details] [diff] [review] > Patch > > "toolbaritem-with-separator" suggests that there will be one separator in > total, but there can be multiple separators, the exact number being > dependent on the number of buttons. Please come up with a better class name. Talked over IRC: <jaws> dao: what do you think of "toolbaritem-combined-buttons" instead of "toolbaritem-with-separator" ? <dao> jaws: that sounds better

https://hg.mozilla.org/projects/ux/rev/859ff4a1af98 (In reply to :Gijs Kruitbosch from comment #3) > Comment on attachment 814415 [details] [diff] [review] > Patch > > Review of attachment 814415 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me, but we should probably make sure that we deal with OS X and Linux as > well. Updated the OSX styling. Since Linux doesn't have the combined toolbar button styling yet there weren't any changes to make for Linux. That work is tracked in bug 875479.