Closed Bug 1436445 Opened 2 years ago Closed 2 years ago
Use correct icon for Firefox Snap
From Ubuntu: I just noticed the icon in the desktop file is actually using the image installed from the deb. This leaves firefox with a missing icon if you only have the snap installed. Here's a patch that will use the icon included in your snap.
Comment on attachment 8949088 [details] Bug 1436445 - Use icon included in the snap. https://reviewboard.mozilla.org/r/218494/#review225134 We have control over firefox.desktop. I'd prefer to manually edit , instead of using a sed command. I'll put the patch up, to save some time.  https://searchfox.org/mozilla-central/rev/b9f1a4ecba48b2d8c686669e32d109c40e927b48/taskcluster/docker/firefox-snap/firefox.desktop#157
Attachment #8949088 - Flags: review?(jlorenzo)
I see we're using the 128x128 icon, which is okay as a first fix. Do you know how can we let Gnome decide which icon to use (depending on the screen resolution), Ken?
AFAIK we cannot specify a generic icon name and let GNOME pick the right resolution, because the icon has to be embedded inside the snap, and the icon lookup algorithm (https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#icon_lookup) isn't snap-aware. We used that 128×128 icon because that's the highest resolution available in the snap, but if the build had a 512×512 icon we could use that, and assume the shell can downscale properly where needed.
Thanks for the background! We don't appear to have a 512*512 icon versioned in-tree. I just asked it in bug 1437478. I see we have a 256*256 icon, but it's not included in the final Linux build. Bug 1437478 asks about it too.  https://searchfox.org/mozilla-central/source/browser/branding/official/default256.png
Comment on attachment 8950154 [details] Bug 1436445 - Use icon included in the snap https://reviewboard.mozilla.org/r/219414/#review225222
Attachment #8950154 - Flags: review?(mozilla) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/61d324b24f98 Use icon included in the snap r=mkaply
Comment on attachment 8950154 [details] Bug 1436445 - Use icon included in the snap Approval Request Comment [Feature/Bug causing the regression]: Needed for proper icon in our Snap [User impact if declined]: incorrect icon [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: [Is the change risky?]: No [Why is the change risky/not risky?]: Only applies to our Snap file. [String changes made/needed]:
Attachment #8950154 - Flags: approval-mozilla-beta?
Comment on attachment 8950154 [details] Bug 1436445 - Use icon included in the snap Looks fine to uplift. This should land in time for beta 12.
Attachment #8950154 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Wait, this already landed on beta (Comment 9)? Well, ok. Is there anything left to do here?
I completely missed that this landed. Sorry about that.
You need to log in before you can comment on or make changes to this bug.