Closed Bug 1374474 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: Theme, defect, P1)

55 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.1 - Jun 26
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- verified
firefox56 --- verified

People

(Reporter: mattsmeltz, Assigned: johannh)

References

Details

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

Attachments

(2 files)

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.
Component: Untriaged → Theme
regression from bug 1352364, that removes "opacity: .25;" rule
  https://hg.mozilla.org/mozilla-central/rev/d72b590f31e3#l7.686
Blocks: 1352364
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
also bug 1362408 that adds "opacity: 1 !important;" rule.
Blocks: 1362408
Could you please attach a screenshot of what you're seeing?
Flags: needinfo?(mattsmeltz)
Whiteboard: [photon-visual][triage]
Johann, can you please look into this?
Flags: needinfo?(mattsmeltz) → needinfo?(jhofmann)
Yeah. I can confirm this issue. I'll take a look at it.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
Iteration: --- → 56.1 - Jun 26
Flags: qe-verify+
Priority: -- → P1
QA Contact: brindusa.tot
Whiteboard: [photon-visual][triage] → [photon-visual]
Duplicate of this bug: 1374619
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 on attachment 8879543 [details]
Bug 1374474 - Add context-fill-opacity to back-large.svg.

https://reviewboard.mozilla.org/r/150838/#review155626
Attachment #8879543 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0d94dbc1b955
Add context-fill-opacity to back-large.svg. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/0d94dbc1b955
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Please request uplift approval ASAP.
Flags: needinfo?(jhofmann)
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
Flags: needinfo?(jhofmann)
Attachment #8879543 - Flags: approval-mozilla-beta?
Blocks: 1367015
No longer blocks: 1352364
Comment on attachment 8879543 [details]
Bug 1374474 - Add context-fill-opacity to back-large.svg.

fix back button styling on mac, beta55+
Attachment #8879543 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.