1413568 - Installer is using the old icon

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Attachment: Screenshot-2017-11-1 v3q0vhx0v9vz png (PNG Image, 224 × 230 pixels).png

Firefox 57 introduced a new Firefox icon. According to https://www.reddit.com/r/firefox/comments/7a0p6x/dont_forget_to_update_the_installer_icon_for_v57/ , we're not using it.

We should use it.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Requesting tracking, since I imagine we'll want some icon consistency here.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(Presumably, Firefox 58 is also affected)

Comment 3

[Molly Howell (she/her) (no longer active)]

Just to manage expectations for anyone who hasn't been involved with this before, I want to point out that, because of how the installer is built, we can only have one icon across all channels; currently this means that the release/beta logo is used even on nightly and dev edition installers, and my guess is that that will remain the case here.

Comment 4

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

Tracked as a 57 blocker.

Comment 5

[Justin Dolske [:Dolske]]

Previously updated in bug 1366763. Matt says that was also meant to update other-licenses/7zstub/src/7zip/Bundles/SFXSetup-moz/setup.ico, so let's do that this time. :)

Comment 6

[Mike Conley (:mconley) (:⚙️)]

I talked to mart3ll today, who said he could produce the new icon for us. Setting needinfo? on him for the asset.

Comment 7

[Stephen Horlander [:shorlander]]

Attachment: Firefox Installer.ico

New Installer Icon.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Outstanding, thanks!

mhowell, got everything you need?

Comment 9

[Stephen Horlander [:shorlander]]

Attachment: Firefox Installer.ico

Slight updated

Comment 10

[Molly Howell (she/her) (no longer active)]

That's perfect; all the right sizes are in there and they all look great. Thanks, Stephen!

Mike, I do need a real quick review on this; could I trouble you for that?

Comment 11

[Molly Howell (she/her) (no longer active)]

Attachment: Bug 1413568 - Update installer icons

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

Comment 12

[Mike Conley (:mconley) (:⚙️)]

I'm happy to rubber-stamp this, but I'll be flat-out honest; I don't really know how the installer works, and can't say with a high degree of certainty that you're changing all of the right files in all of the right places.

I am, however, willing to believe that _you_ are such an expert, and to vouch for you with an rs=me. :)

Comment 13

[Molly Howell (she/her) (no longer active)]

Hey, that's all I need. ;)

If you do want to verify the patch yourself as quickly as possible, you can run `mach build installer` with an artifact build and make sure the installer at $OBJDIR/dist/install/sea/ is using the new icon.

Comment 14

[Molly Howell (she/her) (no longer active)]

Attachment: Patch - update installer icons

rs+ granted by mconley in comment 12

Comment 15

[Nicole Yee (:nicoleyee)]

Hey Matt,
I also trust that this is working just perfectly, but given that it is 57, jwilliams is here to verify the bug when it lands :) Looks like we can cover all our bases since this is late uplift.

Comment 16

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8924598 [details] [diff] [review]
Patch - update installer icons

Will push to inbound once it reopens. Might as well go ahead and fill out the uplift request.

Approval Request Comment
[Feature/Bug causing the regression]:
N/A

[User impact if declined]:
Noticeably inconsistent branding (in fact this bug originates from a reddit thread, so users already noticed).

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Too soon for it to be in a real nightly build, but I've verified it locally.

[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 only replaces icon resources, there are no changes to any code.

[String changes made/needed]:
None

Comment 17

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8924593 [details]
Bug 1413568 - Update installer icons

https://reviewboard.mozilla.org/r/195826/#review201026

I tested this patch as suggested by using an artifact build on my Windows 7 VM, and using `mach build installer1.

I can confirm that the new icon is present! Thanks for the quick work, mhowell and shorlander!

Comment 18

[Molly Howell (she/her) (no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d063578fc2699975f1bbc67a7190bf3fcc5494aa
Bug 1413568 - Update installer icons. r=mconley

Comment 19

[Molly Howell (she/her) (no longer active)]

Awesome, thanks mconley!

Comment 20

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

Comment on attachment 8924598 [details] [diff] [review]
Patch - update installer icons

Must fix, Beta57+

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/48d9303105bd (FIREFOX_57b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/2d2c00ece76c

Comment 22

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

https://hg.mozilla.org/mozilla-central/rev/d063578fc269

Comment 23

[Justin [:JW_SoftvisionQA]]

Every thing looks good from QA. I noticed with Beta the installer may show as the old green installer. I contacted :mhowell and he informed me that may be a cache issue and to look in the installer properties which showed as the new blue installer. Marking as verified on 57 and 58.

Comment 25

[Will]

I think this is the best place to post this because it's related: it seems during the install process, the old logo is being used in some places , look at this screenshot https://twitter.com/bdsams/status/930437694135717893 didn't someone go through the full first-time install process to make sure the logo is updated to the new one?

Comment 26

[Molly Howell (she/her) (no longer active)]

Given when that tweet was posted, that would be bug 1417066, which was fixed a couple of hours after that.

Comment 27

[Will]

Since Firefox stable gets a new icon (again) on the 22nd, has the installer icon been updated too?

Comment 28

[Molly Howell (she/her) (no longer active)]

Yes, we did that in bug 1578489.

Comment 29

[Will]

Awesome! Also (and I apologize because I know you're not really supposed to bring up unrelated bugs) someone forgot to update the "New private window" icon in the Windows jumplist, it's using the very old Private icon :( can someone finally do this? https://bugzilla.mozilla.org/show_bug.cgi?id=1292348

Comment 30

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.