1365002 - The back button's :active:not(:hover) state looks unintentional

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Markus Stange [:mstange]]

If you hold down the back button and move the mouse away from it, it ends up in a state that doesn't look like it was intentionally designed.

Is there an :active selector somewhere that should be an :active:hover selector instead?

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1365002 - Don't remove the back button background on state change.

This CSS rule made certain actions on MacOS look strange, e.g.
active without hover, and it doesn't appear to be needed on the
other platforms since the background is always overriden when needed.

Review commit: https://reviewboard.mozilla.org/r/139722/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139722/

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8868110 [details]
Bug 1365002 - Don't remove the back button background on state change.

https://reviewboard.mozilla.org/r/139722/#review143428

::: browser/themes/shared/toolbarbuttons.inc.css:371
(Diff revision 1)
>    padding: 7px !important;
>  }
>  
>  %ifdef MOZ_PHOTON_THEME
> -#back-button:not(:hover):not(:active):not([open=true]) > .toolbarbutton-icon,
> -#back-button[disabled=true] > .toolbarbutton-icon {
> +#back-button > .toolbarbutton-icon {
> +  background: var(--backbutton-background);

Please merge this into the previous #back-button > .toolbarbutton-icon rule.

Comment 3

[Johann Hofmann [:johannh]]

Comment on attachment 8868110 [details]
Bug 1365002 - Don't remove the back button background on state change.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139722/diff/1-2/

Comment 4

[Johann Hofmann [:johannh]]

Updated.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8868110 [details]
Bug 1365002 - Don't remove the back button background on state change.

https://reviewboard.mozilla.org/r/139722/#review143436

::: browser/themes/shared/toolbarbuttons.inc.css:372
(Diff revision 2)
>  
>  #back-button > menupopup {
>    margin-top: -1px !important;
>  }
>  
>  #back-button > .toolbarbutton-icon {

I meant this rule, though.

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 8868110 [details]
Bug 1365002 - Don't remove the back button background on state change.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139722/diff/2-3/

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8868110 [details]
Bug 1365002 - Don't remove the back button background on state change.

https://reviewboard.mozilla.org/r/139722/#review143444

::: browser/themes/shared/toolbarbuttons.inc.css:376
(Diff revision 3)
>  }
>  
>  #back-button > .toolbarbutton-icon {
>  %ifdef MOZ_PHOTON_THEME
>    border-color: var(--backbutton-border-color) !important;
> +  background: var(--backbutton-background);

Please move background up to keep border-color and border-radius together.

Comment 8

[Johann Hofmann [:johannh]]

Comment on attachment 8868110 [details]
Bug 1365002 - Don't remove the back button background on state change.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139722/diff/3-4/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf6b08540d0c
Don't remove the back button background on state change. r=dao

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bf6b08540d0c

Comment 11

[Dão Gottwald [:dao]]

(In reply to Johann Hofmann [:johannh] from comment #1)
> Created attachment 8868110 [details]
> Bug 1365002 - Don't remove the back button background on state change.
> 
> This CSS rule made certain actions on MacOS look strange, e.g.
> active without hover, and it doesn't appear to be needed on the
> other platforms since the background is always overriden when needed.

Have you checked what made Mac behave differently? Is there some obsolete rule in browser.css that we should remove?

Comment 12

[Johann Hofmann [:johannh]]

(In reply to Dão Gottwald [::dao] from comment #11)
> (In reply to Johann Hofmann [:johannh] from comment #1)
> > Created attachment 8868110 [details]
> > Bug 1365002 - Don't remove the back button background on state change.
> > 
> > This CSS rule made certain actions on MacOS look strange, e.g.
> > active without hover, and it doesn't appear to be needed on the
> > other platforms since the background is always overriden when needed.
> 
> Have you checked what made Mac behave differently? Is there some obsolete
> rule in browser.css that we should remove?

I don't think so, IIRC it's simply that the translucent back button didn't actually look different on the other platforms because of their toolbar background.

Comment 13

[ovidiu boca[:Ovidiu]]

I verified this issue on Mac OS X 10.12 with Nightly 57.0a1(2017-08-04) and I can confirm the fix.