I've noticed that the active buttons have the normal hover effect when hovered, which makes it confusing when you click a toggle-able button like on the console. Just need to do a little cleanup before we move on to Bug 952100.

Another issue (from Bug 966200): 1. Enable devtools.debugger.tracer 2. Open debugger The stepping buttons have a different border color vs. the tracer button. I think all the buttons should have the same border. The [O|Import] button in the profiler has a darker border than the rest of the buttons.

The issue with groups having different borders has been handled in debugger.inc.css and profiler.inc.css. We should really have a 'devtools-button-group' class for this type of pattern so it can be reused, but this resolves the issue in its current state. Also fixes the [checked]:hover issue on light theme buttons by applying certain rules specifically to the dark theme in toolbars.inc.css and moves some of the shared rules above the specific theme overrides.

Comment on attachment 8368587 [details] [diff] [review] theme-button-follow-up.patch Review of attachment 8368587 [details] [diff] [review]: ----------------------------------------------------------------- Is there a followup for creating a shared devtools-button-group class? If so, mind adding the bug in comments in the css files? If not, would it be super hard to fix it now? (I don't feel strongly either way) ::: browser/themes/shared/devtools/toolbars.inc.css @@ +106,4 @@ > background-color: transparent !important; > } > > +.theme-dark .devtools-toolbarbutton[checked=true]:hover:active { Do we need .theme-light equivalents for these colors? If so (and they're in the file, care to move them here?

(In reply to Victor Porof [:vp] from comment #4) > Comment on attachment 8368587 [details] [diff] [review] > theme-button-follow-up.patch > > Review of attachment 8368587 [details] [diff] [review]: > ----------------------------------------------------------------- > > Is there a followup for creating a shared devtools-button-group class? If > so, mind adding the bug in comments in the css files? If not, would it be > super hard to fix it now? (I don't feel strongly either way) > I've updated the patch to go ahead and make the helper class and remove specific code for each group. I've asked for review again just to cover these changes. > ::: browser/themes/shared/devtools/toolbars.inc.css > @@ +106,4 @@ > > background-color: transparent !important; > > } > > > > +.theme-dark .devtools-toolbarbutton[checked=true]:hover:active { > > Do we need .theme-light equivalents for these colors? If so (and they're in > the file, care to move them here? In this case, the color is defined alongside a bunch of others: .theme-light .devtools-menulist[open=true], .theme-light .devtools-toolbarbutton[open=true], .theme-light .devtools-toolbarbutton[open=true]:hover, .theme-light .devtools-toolbarbutton[open=true]:hover:active, .theme-light .devtools-toolbarbutton[checked=true], .theme-light .devtools-toolbarbutton[checked=true]:hover, .theme-light .devtools-toolbarbutton[checked=true]:hover:active The button UI (for both themes) is going to be changed in Bug 952100, so some of this will probably change soon.

https://hg.mozilla.org/integration/fx-team/rev/ce2e31b34b1e https://tbpl.mozilla.org/?tree=Fx-Team&rev=ce2e31b34b1e