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

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Edo Karabas]

Attachment: 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.

Comment 1

[Edo Karabas]

[Tracking Requested - why for this release]: small, but visible regression in one of the default themes

Comment 2

[Johann Hofmann [:johannh]]

I agree this is annoying, Sean or Stephen, is there any chance we can get the accent color updated in time for 57?

Comment 3

[Stephen Horlander [:shorlander]]

It should work if we change the accentcolor to #834d29

Sean should update it on AMO also though.

Comment 4

[Johann Hofmann [:johannh]]

Attachment: Bug 1404108 - Change the accent color for the Web Browser Renaissance theme.

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

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

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?

Comment 6

[Johann Hofmann [:johannh]]

(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 7

[Johann Hofmann [:johannh]]

Comment on attachment 8920202 [details]
Bug 1404108 - Change the accent color for the Web Browser Renaissance theme.

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

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

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!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03395b68497f
Change the accent color for the Web Browser Renaissance theme. r=nhnt11

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/03395b68497f

Comment 11

[Julien Cristau [:jcristau]]

Do we want this on 57?  If so please request uplift ASAP.

Comment 12

[Sean Martell]

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.

Comment 13

[Edo Karabas]

Is there anything else to do ? The issue is resolved for me with the latest Nightly and it should be definitely uplifted to 57.

Comment 14

[Andy McKay]

(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 15

[Johann Hofmann [:johannh]]

Attachment: Patch for uplift

Comment 16

[Johann Hofmann [:johannh]]

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

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8922330 [details] [diff] [review]
Patch for uplift

Recent regression, low risk fix, Beta57+

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d87eb6ee9457

Comment 19

[roxleitan]

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