Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing icon sizes #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leoheitmannruiz
Copy link
Contributor

@leoheitmannruiz leoheitmannruiz commented Jul 17, 2024

If Firefox doesn't encounter the icon size it expects, it fetches the icon from addons.mozilla.org.

This is especially annoying on a slow connection or when not connected at all. But generally, I'd just rather have it not download the icon.

This should add all sizes.

magick icon128.png -resize 16x16 icon16.png
magick icon128.png -resize 32x32 icon32.png
magick icon128.png -resize 64x64 icon64.png
magick icon128.png -resize 96x96 icon96.png

zopflipng -y icon16.png icon16.png
zopflipng -y icon32.png icon32.png
zopflipng -y icon64.png icon64.png
zopflipng -y icon96.png icon96.png

@tom-james-watson
Copy link
Owner

Hi!

Firefox is using manifest-v2, so that would need updating too. Manifest-v3 is Chrome-only at the moment.

Where are you getting this list of sizes from? When are these images used and when are you seeing a slow down because of that?

@leoheitmannruiz
Copy link
Contributor Author

Yeah, so, hehe

I think I've made this a lot more complicated than it should have, sorry.

The icon situation is Firefox specific, which is why I first added the icons to manifest-v2.

But then I sent my other PR, and thought you'd use the V3 version for both Chrome and Firefox, so I updated this PR and force-pushed. I suppose that was a little silly :)

Also, I edited my description. The original description was a lot better. Silly me again.

A fresh start:

If Firefox doesn't encounter the icon size it wants, it fetches the icon from https://addons.mozilla.org/user-media/addon_icons/*

On my 2022 MacBook Air, this happens at about:addons (at most zoom levels, including 100%) and in the extension pop up thingie (the puzzle piece).

Normally, if Firefox doesn't encounter the icon size it wants, it downloads it once and then it's got it. However, I clear my browsers cache regularly (that is, when closing my browser). As a result, often when I visit about:addons or click the puzzle piece, the icon is fetched again.

On a slow connection this is very notable, as the other icons are displayed instantly, while the Old Reddit Redirect icon takes a moment. And when I'm not connected at all, there's no icon.

Where are you getting this list of sizes from?

Mostly by testing all zoom levels, while not connected, and checking what URL Firefox wants to get the icon from (by clicking "open in new tab" where there should have been an icon):

And, checking what other extensions do:

The whole extension icon situation at Firefox sadly seems a little weird.

Something I had in my original description:

I can imagine this being an annoying PR, especially as it ads ~10KB to a ~20KB add-on. Hope it's fine ^^

Alternatively, a single SVG file can be used for all sizes (though, only for Firefox). Do you perhaps have one?

If you want to, I can try and recreate the icon as an SVG, though, of course, it's going to differ ever so slightly.

@leoheitmannruiz
Copy link
Contributor Author

I changed the commit back to adding the icons to manifest-v2.

@tom-james-watson
Copy link
Owner

Ahh ok, thanks for the explanation! I will leave this for later and merge this and do the switch for manifest-v3 only. I want to let the other version publish and go out first, plus I have some holidays coming up, so will probably not be for a few weeks.

@leoheitmannruiz
Copy link
Contributor Author

Cool! Enjoy your holiday!!

@leoheitmannruiz
Copy link
Contributor Author

How's it looking with this and the manifest v3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants