Using a manifest that does not define any icon results in a default extension icon being used. After bug 1395387 this no longer happens for page actions.
This re-introduces getIconData which was removed in bug 1395387. The trick here is that getPreferredIcon actually is where we add the default extension icon if the manifest does not contain the icon. We also limited to 16/32 for icons at this point. getIconData is different than before in that it returns an object usable by PageActions rather than the css we used to return.
Comment on attachment 8924285 [details] Bug 1413648 fix default extension icon use, lgtm ftw! I should have done it this way. :-) Here's the additional review you asked for, but take it with a grain of salt, or as an f+, since I'm not a webextensions peer or anything.
Attachment #8924285 - Flags: review+
Comment on attachment 8924285 [details] Bug 1413648 fix default extension icon use, https://reviewboard.mozilla.org/r/195502/#review200818 How about adding a test too?
Attachment #8924285 - Flags: review?(aswan) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/6bc39f03ea57 fix default extension icon use, r=aswan
I can reproduce this issue on Firefox 58.0a1 (20171101104430) under Wind 7. As you can see in the screenshot, the icon is not displayed for the default_icon. On Firefox 59.0a1 (20171115220414) under Wind 7 the icon is displayed if you have the default_icon removed from the manifest.json file but in case that default_icon has an empty string, the icon is not displayed. Is this expected? Or I should log another bug for this issue?
bug 1419109 for the empty icon. This is not caused by the changes in this bug (or the related bugs), it happened prior.
You need to log in before you can comment on or make changes to this bug.