Closed Bug 1413568 Opened 2 years ago Closed 2 years ago
Installer is using the old icon
17.48 KB, image/png
57.35 KB, image/x-icon
207.01 KB, patch
|Details | Diff | Splinter Review|
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.
Requesting tracking, since I imagine we'll want some icon consistency here.
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.
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. :)
I talked to mart3ll today, who said he could produce the new icon for us. Setting needinfo? on him for the asset.
New Installer Icon.
Outstanding, thanks! mhowell, got everything you need?
Attachment #8924586 - Attachment is obsolete: true
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?
Flags: needinfo?(mhowell) → needinfo?(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. :)
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.
rs+ granted by mconley in comment 12
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 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
Attachment #8924598 - Flags: approval-mozilla-beta?
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!
Attachment #8924593 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d063578fc2699975f1bbc67a7190bf3fcc5494aa Bug 1413568 - Update installer icons. r=mconley
Awesome, thanks mconley!
Comment on attachment 8924598 [details] [diff] [review] Patch - update installer icons Must fix, Beta57+
Attachment #8924598 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
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?
Given when that tweet was posted, that would be bug 1417066, which was fixed a couple of hours after that.
You need to log in before you can comment on or make changes to this bug.