default icon no longer appears for page actions

VERIFIED FIXED in Firefox 58

Status

defect
P2
normal
VERIFIED FIXED
2 years ago
11 months ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

({regression})

49 Branch
mozilla58
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57 unaffected, firefox58 verified, firefox59 verified)

Details

Attachments

(2 attachments)

Assignee

Description

2 years ago
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.
Assignee

Updated

2 years ago
Priority: -- → P2

Updated

2 years ago
Keywords: regression
Comment hidden (mozreview-request)
Assignee

Comment 2

2 years ago
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 4

2 years ago
mozreview-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+
Comment hidden (mozreview-request)

Comment 6

2 years ago
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6bc39f03ea57
fix default extension icon use, r=aswan

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6bc39f03ea57
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 8

2 years ago
Posted file Bug1413648.zip
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?
Flags: needinfo?(mixedpuppy)
Assignee

Updated

2 years ago
See Also: → 1419109
Assignee

Comment 9

2 years ago
bug 1419109 for the empty icon.  This is not caused by the changes in this bug (or the related bugs), it happened prior.
Flags: needinfo?(mixedpuppy)

Updated

2 years ago
Assignee: nobody → mixedpuppy

Comment 10

2 years ago
Based on comment 8 and comment 9, this issue is verified as fixed on Firefox 59.0a1 (20171120222519) and Firefox 58.0b4 (20171115114231) under Wind 7 64-bit and Mac OS X 10.13.
Status: RESOLVED → VERIFIED

Updated

11 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.