1362408 - Disabled back button is too translucent

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Markus Stange [:mstange]]

[Tracking Requested - why for this release]: unintended visual regression in the primary UI

The back button's background's opacity changes when the button becomes disabled / enabled. This looks broken. Every new tab starts out with a disabled back button, so this happens all the time.

On Mac, native buttons keep their full opacity when they become disabled and only the glyph or text on top of them becomes less opaque. We should match that, and we did, until the recent changes.

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1362408 - Fix disabled back button opacity on OSX.

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

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8866298 [details]
Bug 1362408 - Fix disabled back button opacity on OSX.

https://reviewboard.mozilla.org/r/137920/#review141038

::: browser/themes/osx/browser.css:21
(Diff revision 1)
>  
>  :root {
>    --space-above-tabbar: 9px;
>    --tabs-toolbar-color: #333;
> +  /* This should be the same as --toolbarbutton-icon-fill, with reduced opacity. */
> +  --toolbarbutton-icon-fill-disabled: rgba(76, 76, 76, 0.4);

This seems rather fragile, and is in fact wrong with dark lightweight themes unless I'm missing something.

Comment 3

[Dão Gottwald [:dao]]

jwatt, http://searchfox.org/mozilla-central/search?q=context-fill-opacity suggests that context-fill-opacity exists in theory but isn't currently supported. What would it take to enable this?

Comment 4

[Julien Cristau [:jcristau] (back April 22)]

tracking as ui regression in 55.

Comment 5

[Johann Hofmann [:johannh]]

Short term we could also just override this variable for dark lightweight theme and add a TODO comment until context-fill-opacity gets enabled.

Comment 6

[Dão Gottwald [:dao]]

We're in no rush -- this can wait until we can fix it the right way.

Comment 7

[Jonathan Watt [:jwatt]]

(In reply to Dão Gottwald [::dao] from comment #3)
> jwatt, http://searchfox.org/mozilla-central/search?q=context-fill-opacity
> suggests that context-fill-opacity exists in theory but isn't currently
> supported. What would it take to enable this?

I've opened bug 1365926 to take a look at this.

Comment 8

[Jonathan Watt [:jwatt]]

The code fixes for bug 1365926 just landed, but there won't be a notification here since the bug won't be closed just yet until I wrap up the tests. You can still implement the alternative 'context-fill-opacity' based fix for this bug now though.

Comment 9

[Johann Hofmann [:johannh]]

Comment on attachment 8866298 [details]
Bug 1362408 - Fix disabled back button opacity on OSX.

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

Comment 10

[Johann Hofmann [:johannh]]

(In reply to Jonathan Watt [:jwatt] from comment #8)
> The code fixes for bug 1365926 just landed, but there won't be a
> notification here since the bug won't be closed just yet until I wrap up the
> tests. You can still implement the alternative 'context-fill-opacity' based
> fix for this bug now though.

Thanks!

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8866298 [details]
Bug 1362408 - Fix disabled back button opacity on OSX.

https://reviewboard.mozilla.org/r/137920/#review150170

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1dd8e8368519
Fix disabled back button opacity on OSX. r=dao

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/1dd8e8368519

Comment 14

[Markus Stange [:mstange]]

Thank you for fixing this!

Comment 15

[ovidiu boca[:Ovidiu]]

I tested this on Windows 10x 64 with Nightly 57.0a1(2017-08-09) and the back button background changes if the button is disabled or enabled. If it's disabled it becomes grayed out. Please tell me if this is this works as intended? Thanks

Comment 16

[Johann Hofmann [:johannh]]

(In reply to ovidiu boca[:Ovidiu] from comment #15)
> I tested this on Windows 10x 64 with Nightly 57.0a1(2017-08-09) and the back
> button background changes if the button is disabled or enabled. If it's
> disabled it becomes grayed out. Please tell me if this is this works as
> intended? Thanks

You only need to test this on 55, where the disabled back button should not look different than the one in 54.

Comment 17

[ovidiu boca[:Ovidiu]]

Attachment: back button.png

Please see the attached file. This is on Mac Os X 10.10 with FF 55 and FF 54, both back buttons are disabled.  From my point of view, the back button doesn't look the same, but please tell me your opinion. Thanks

Comment 18

[Johann Hofmann [:johannh]]

The point is that the button background should be the same, when the window is focused. In this screenshot one window is not focused.

Comment 19

[ovidiu boca[:Ovidiu]]

Thanks for clearing this out, you are right, the button background is the same on FF 54 and FF 55, Mac OS X 10.10.