Closed Bug 1362408 Opened 3 years ago Closed 3 years ago

Disabled back button is too translucent

Categories

(Firefox :: Theme, defect, P1)

All
macOS
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.7 - Jun 12
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 + verified

People

(Reporter: mstange, Assigned: johannh)

References

Details

(Keywords: regression, Whiteboard: [photon-visual][p2])

Attachments

(2 files)

[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.
Blocks: 1352364
Whiteboard: [photon-visual][p2]
Flags: qe-verify?
Priority: -- → P2
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.
Attachment #8866298 - Flags: review?(dao+bmo) → review-
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?
Flags: needinfo?(jwatt)
tracking as ui regression in 55.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Iteration: --- → 55.5 - May 15
Priority: P2 → P1
Short term we could also just override this variable for dark lightweight theme and add a TODO comment until context-fill-opacity gets enabled.
We're in no rush -- this can wait until we can fix it the right way.
Iteration: 55.5 - May 15 → 55.6 - May 29
Flags: qe-verify? → qe-verify+
QA Contact: brindusa.tot
Depends on: 1365926
(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.
Flags: needinfo?(jwatt)
Iteration: 55.6 - May 29 → 55.7 - Jun 12
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.
Flags: needinfo?(jhofmann)
(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!
Flags: needinfo?(jhofmann)
Comment on attachment 8866298 [details]
Bug 1362408 - Fix disabled back button opacity on OSX.

https://reviewboard.mozilla.org/r/137920/#review150170
Attachment #8866298 - Flags: review?(dao+bmo) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1dd8e8368519
Fix disabled back button opacity on OSX. r=dao
https://hg.mozilla.org/mozilla-central/rev/1dd8e8368519
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Thank you for fixing this!
Depends on: 1374474
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
Flags: needinfo?(jhofmann)
(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.
Flags: needinfo?(jhofmann)
Attached image 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
The point is that the button background should be the same, when the window is focused. In this screenshot one window is not focused.
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.