1627663 - Toolbox: the buttons don't have a hover effect anymore

Status: RESOLVED FIXED

(DevTools :: General, defect, P3)

Description

[Julien Wajsberg [:julienw]]

This seems new from several days: we don't see hover effects on the toolbox buttons anymore (inspect, settings, tool buttons, close, ...).

STR:

1. Open the devtools;
2. Hover one of these buttons.
   => see that the background color doesn't change.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:Honza, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the report Julien!

Honza

Comment 3

[Alexandru Trif, Desktop QA [:atrif]]

Hello, attaching the regression range made on Windows 10x64:
Last good revision: f6d03ead730474725aa8f835b6b602f094764bc5
First bad revision: bd5ca045d481384cf7e1de5d71b70e57e9b68ef5
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f6d03ead730474725aa8f835b6b602f094764bc5&tochange=bd5ca045d481384cf7e1de5d71b70e57e9b68ef5

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks Alexandru!

Florens, do you know what's the issue here?

Honza

Comment 5

[Florens Verschelde :fvsch]

Not sure yet, only part of the changeset that seems relevant is:

   /* Toolbar buttons */
-  --toolbarbutton-background: var(--theme-toolbar-hover);
+  --toolbarbutton-background: var(--theme-toolbar-background);
   --toolbarbutton-hover-background: var(--theme-toolbar-hover);

I think the change was needed to harmonize the background-color and hover background-color of accordion header.

I'll have to test how this variable may be used or not for toolbox buttons in different states and for the light and dark theme (we're sometimes not consistent).

Our button styles are mess because over time we shared similar-but-different styles by piling on selectors, something like:

.devtools-button,
.devtools-toolbarbutton,
.devtools-othertypeofbutton {
  ...
}

then we have styles where we want the same colors but for different states depending on the button type, using a theme variable whose name doesn't match each use case:

.devtools-button:hover,
.devtools-toolbarbutton:hover:active,
.devtools-othertypeofbutton[aria-pressed="true"] {
  background-color: var(--theme-toolbar-hover-active-background);
}

that kind of thing :)

IMO the fix would be to de-tangle some of those styles and/or variables, so that we can tweak the style of one type of button without affecting other buttons, which is important for background colors because we tend to use solid colors for buttons and need to coordinate that with the background color of the context where the button is used (a problem in and of itself).

Keeping the needinfo so that I can double-check how this variable impacts the toolbox buttons.

Comment 6

[Florens Verschelde :fvsch]

Looks like we have this style:

.devtools-toolbarbutton:not([label]):hover,
.devtools-button:empty:not(:disabled):not([aria-expanded="true"]):hover {
    background: var(--toolbarbutton-background);
}

and I'm not sure why it takes precedence over this:

.devtools-button:not(:empty):not(:disabled):not(.checked):hover,
.devtools-toolbarbutton[label]:not(:-moz-any([checked=true],[disabled])):hover,
.devtools-button:empty:not(:disabled):-moz-any(:hover:active,.checked),
.devtools-toolbarbutton:not([label]):-moz-any([checked],[open],:hover:active),
.devtools-button[aria-haspopup="menu"][aria-expanded="true"] {
  background-color: var(--toolbarbutton-hover-background);
}

we can try to untangle those styles, but we risk breaking other stuff elsewhere.

Safest and cleanest option IMO is to remove the devtools-button class from the toolbox toolbar buttons, and create styles specific to the toolbox toolbar buttons.

Comment 7

[Florens Verschelde :fvsch]

Attachment: Bug 1627663 - Add separate devtools-tabbar-button style for toolbox buttons; r=Honza

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/992c64b7f3e3
Add separate devtools-tabbar-button style for toolbox buttons; r=Honza

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/992c64b7f3e3