Closed Bug 1404108 Opened 3 years ago Closed 3 years ago

No visible highlighting color of the active tab for a Web Browser Renaissance theme

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- verified
firefox58 --- verified

People

(Reporter: Eddwardiq, Assigned: johannh)

References

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(3 files)

Attached image no accent color.png
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20170928100123

Steps to reproduce:

Since Web Browser Renaissance theme appears to be an option for choose by default it would be nice to have an accent color of the active tab in this theme. Right now there is none / invisible.
It is reproducible on the latest Nightly as well as Firefox 57 beta 3. Using Windows 10. It does not depend on a density option.
Blocks: 1388138
Component: Untriaged → Theme
Whiteboard: [photon-visual][triage]
[Tracking Requested - why for this release]: small, but visible regression in one of the default themes
I agree this is annoying, Sean or Stephen, is there any chance we can get the accent color updated in time for 57?
Flags: needinfo?(smartell)
Flags: needinfo?(shorlander)
Flags: qe-verify?
Priority: -- → P5
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
Flags: qe-verify? → qe-verify+
It should work if we change the accentcolor to #834d29

Sean should update it on AMO also though.
Flags: needinfo?(shorlander)
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: P5 → P1
Comment on attachment 8920202 [details]
Bug 1404108 - Change the accent color for the Web Browser Renaissance theme.

https://reviewboard.mozilla.org/r/191248/#review197008

Almost r+'d, but I had a passing thought: if Web Browser Renaissance is the currently selected theme, will the migration code's accent color update immediately take effect? Or will the user be stuck with the old accent color till restart?
Attachment #8920202 - Flags: review?(nhnt11)
(In reply to Nihanth Subramanya [:nhnt11] from comment #5)
> Comment on attachment 8920202 [details]
> Bug 1404108 - Change the accent color for the Web Browser Renaissance theme.
> 
> https://reviewboard.mozilla.org/r/191248/#review197008
> 
> Almost r+'d, but I had a passing thought: if Web Browser Renaissance is the
> currently selected theme, will the migration code's accent color update
> immediately take effect? Or will the user be stuck with the old accent color
> till restart?

Updates require restart anyway and I'm under the impression that the UI migration always runs on restarts, so I'd say the answer to your question is: No, it won't require restart. :)
Comment on attachment 8920202 [details]
Bug 1404108 - Change the accent color for the Web Browser Renaissance theme.

https://reviewboard.mozilla.org/r/191248/#review197208

Thanks!
Attachment #8920202 - Flags: review?(nhnt11) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03395b68497f
Change the accent color for the Web Browser Renaissance theme. r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/03395b68497f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Do we want this on 57?  If so please request uplift ASAP.
Flags: needinfo?(jhofmann)
When AMO switched over to using Firefox accounts to login I can no longer access my account. Can't sort out how to update my themes.
Flags: needinfo?(smartell)
Is there anything else to do ? The issue is resolved for me with the latest Nightly and it should be definitely uplifted to 57.
(In reply to Sean Martell from comment #12)
> When AMO switched over to using Firefox accounts to login I can no longer
> access my account. Can't sort out how to update my themes.

Create a Firefox Account with the same email as the one you used before the switch, then log in with that account and you'll get access back. If that doesn't work or you can't remember the account, please contact amo-admins @ mozilla.org, or find us on IRC in #amo and we'll help you out.
Comment on attachment 8922330 [details] [diff] [review]
Patch for uplift

Approval Request Comment
[Feature/Bug causing the regression]: Photon redesign
[User impact if declined]: A Web Browser Renaissance Theme will have no visible accent color
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: I can see it in my Nightly
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: This patch just changes a color value in a pref. The original patch on central also performs a UI migration to update users who currently have the theme activated. To avoid any risks, we would just like to uplift the part of the patch that changes the pref for new users.
[String changes made/needed]: None
Flags: needinfo?(jhofmann)
Attachment #8922330 - Flags: approval-mozilla-beta?
Comment on attachment 8922330 [details] [diff] [review]
Patch for uplift

Recent regression, low risk fix, Beta57+
Attachment #8922330 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reproduced the initial issue using Nightly 58.0a1 (2017-09-29).

Verified fixed using the latest Nightly 58.0a1 (2017-10-26)and FF Beta 57.0b12 on Windows 10 x64, Ubuntu 16.04 x64 and 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.