having "default_icon": "" in the manifest produces buttons with no icon. Happens with page/browser/sidebar actions. We should probably disallow an empty value in the schema, but should also handle that in code. Making this low priority since the extension can address this by not doing that.
Alternatively, one could also provide a default icon (like a jigsaw piece, for starters), no? I don't remember what Chrome does in that case, but would love to further investigate this and work on the implementation.
If this issue is still open I would like to look into fixing it.
(In reply to clintontak22 from comment #2) > If this issue is still open I would like to look into fixing it. Hey there, sorry for the late reply! This issue is still open -- if you're still interested in working on it, please feel welcome to assign it to yourself! If you haven't already done so, you might also want to take a moment to review some tips for onboarding to the Firefox code base: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
Hey Shane! Can you provide some pointers for how a contributor could get started on this bug? If this is your first contribution, please see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp for instructions on how to set up your environment.
I would be interested in taking it up. clintontak22 are you still working on it?
Hey Varun! Since we haven't heard from clintontak22, you're clear to pick this one up! Please needinfo :mixedpuppy if you need any help getting started.
Assignee: nobody → varundey20
Seems like Chrome throws an error if default_icon is empty or one of its key is empty. They also validate if the icon path is correct or not. manifest - https://pastebin.com/MjyQkKng I think it seems the correct way to go.
So I was thinking of putting a validation check for default_icons similar to chrome (throw an error if the file does not exist/is empty) but turns out Firefox does not validate file path for icons as well. Keeping validation check for default_icons but not for icons would look a bit weird IMO.
Assuming the problem is only "default_icon": ""... It probably just needs to be handled in schema. I think it could be done by creating a new NonEmptyExtensionUrl type that is the same as ExtensionUrl, but uses a pattern to ensure it is non-empty, then make IconPath use that. I don't think we want to validate the icons exist during extension startup. We probably only would want to do that during install, if at all. Extension authors should be verifying that their extension works properly.  https://searchfox.org/mozilla-central/rev/f8de61826903996f6bdf41b11a2844dd59ac144f/toolkit/components/extensions/schemas/manifest.json#543
If the extension had either default_icon or one of it's property as an empty string, it would show a black icon in the toolbar. With this patch, it checks if any of default_icon property is empty and throws an error on extension load.
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox66: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Status: RESOLVED → VERIFIED
status-firefox66: fixed → verified
status-firefox67: --- → verified
You need to log in before you can comment on or make changes to this bug.