1374474 - back button in 55 beta does not have greyed out disabled appearance in default theme

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[mws]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170615063752

Steps to reproduce:

1. Use the default theme in 55 beta
2. Navigate to the first page in a tab's session history (or just open a new window or tab)


Actual results:

The back button appearance is not greyed out.


Expected results:

The back button should be greyed out.

Comment 1

[Tooru Fujisawa [:arai]]

regression from bug 1352364, that removes "opacity: .25;" rule
  https://hg.mozilla.org/mozilla-central/rev/d72b590f31e3#l7.686

Comment 2

[Tooru Fujisawa [:arai]]

also bug 1362408 that adds "opacity: 1 !important;" rule.

Comment 3

[Dão Gottwald [:dao]]

Could you please attach a screenshot of what you're seeing?

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: screenshot of new window, on 55.0b2 (64-bit) on osx

Comment 5

[Dão Gottwald [:dao]]

Johann, can you please look into this?

Comment 6

[Johann Hofmann [:johannh]]

Yeah. I can confirm this issue. I'll take a look at it.

Comment 8

[:Gijs (he/him)]

This is because this code:

https://dxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#486-492

overrides the opacity of disabled buttons (which is normally 0.4) with 1 with !important. Instead it uses fill-opacity, but  we don't use that for the back button icon in non-photon, so now it's broken.

Comment 9

[Johann Hofmann [:johannh]]

Attachment: Bug 1374474 - Add context-fill-opacity to back-large.svg.

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

Comment 10

[:Gijs (he/him)]

Comment on attachment 8879543 [details]
Bug 1374474 - Add context-fill-opacity to back-large.svg.

https://reviewboard.mozilla.org/r/150838/#review155626

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d94dbc1b955
Add context-fill-opacity to back-large.svg. r=Gijs

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/0d94dbc1b955

Comment 13

[:Gijs (he/him)]

Please request uplift approval ASAP.

Comment 14

[Johann Hofmann [:johannh]]

Comment on attachment 8879543 [details]
Bug 1374474 - Add context-fill-opacity to back-large.svg.

Approval Request Comment
[Feature/Bug causing the regression]: A combination of bug 1362408 and bug 1367015
[User impact if declined]: Disabled back button does not look disabled on OSX
[Is this code covered by automated tests?]: No, it's just a visual thing
[Has the fix been verified in Nightly?]: Not visible in Nightly
[Needs manual test from QE? If yes, steps to reproduce]: I doesn't require extensive tests, you just need to check that in Beta the back button looks disabled on e.g. about:home.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's just adding a missing style attribute to an SVG file.
[String changes made/needed]: None

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8879543 [details]
Bug 1374474 - Add context-fill-opacity to back-large.svg.

fix back button styling on mac, beta55+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2a236d383461

Comment 17

[ovidiu boca[:Ovidiu]]

Tested this on Mac OS X 10.12 with FF 56 Beta and FF 55 release and I can confirm the fix, the back button looks disabled on about:home page.