empty default icon in manifest produces blank icon in toolbar
Categories
(WebExtensions :: Frontend, enhancement, P5)
Tracking
(firefox66 verified, firefox67 verified)
People
(Reporter: mixedpuppy, Assigned: varundey20, Mentored)
References
Details
(Keywords: good-first-bug)
Attachments
(2 files)
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.
Updated•6 years ago
|
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.
Comment 2•6 years ago
|
||
If this issue is still open I would like to look into fixing it.
Updated•6 years ago
|
Comment 3•6 years ago
|
||
(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
Comment 4•6 years ago
|
||
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.
Assignee | ||
Comment 5•5 years ago
|
||
I would be interested in taking it up. clintontak22 are you still working on it?
Comment 6•5 years ago
|
||
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 | ||
Comment 7•5 years ago
|
||
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.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 8•5 years ago
|
||
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.
Assignee | ||
Updated•5 years ago
|
Reporter | ||
Comment 9•5 years ago
|
||
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. [1] https://searchfox.org/mozilla-central/rev/f8de61826903996f6bdf41b11a2844dd59ac144f/toolkit/components/extensions/schemas/manifest.json#543
Assignee | ||
Comment 10•5 years ago
|
||
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.
Comment 12•5 years ago
|
||
Pushed by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a3bbf490ea41
Fix Bug 1419109 - Empty default icon shows blank icon in toolbar r=mixedpuppy
Comment 13•5 years ago
|
||
Backout for browser-chrome failures at browser/test-oop-extensions/browser_ext_browserAction_pageAction_icon_permissions.js
Backout: https://hg.mozilla.org/integration/autoland/rev/2bb5deb589500edcab98659c4a6659d9bf3e5ffa
Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a3bbf490ea419b2c01a4447d45e7fe0a116144f1
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=221605988&repo=autoland&lineNumber=3217
task 2019-01-13T16:35:00.297Z] 16:35:00 INFO - TEST-PASS | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_browserAction_pageAction_icon_permissions.js | pageAction image removed from document -
[task 2019-01-13T16:35:00.299Z] 16:35:00 INFO - Extension loaded
[task 2019-01-13T16:35:00.303Z] 16:35:00 INFO - Buffered messages finished
[task 2019-01-13T16:35:00.305Z] 16:35:00 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_browserAction_pageAction_icon_permissions.js | Uncaught exception - startup failed
[task 2019-01-13T16:35:00.306Z] 16:35:00 INFO - Leaving test bound testDefaultDetails
[task 2019-01-13T16:35:00.308Z] 16:35:00 INFO - Entering test bound testSecureURLsDenied
[task 2019-01-13T16:35:00.311Z] 16:35:00 INFO - Extension loaded
[task 2019-01-13T16:35:00.313Z] 16:35:00 INFO - Console message: [JavaScript Error: "1547397299640 addons.webextension.<unknown> ERROR Loading extension 'null': Reading manifest: Error processing browser_action.default_icon: Value "/foo/bar.png" must either: be an object value, or match the pattern /^\w/" {file: "resource://gre/modules/Log.jsm" line: 679}]
[task 2019-01-13T16:35:00.319Z] 16:35:00 INFO - append@resource://gre/modules/Log.jsm:679:9
[task 2019-01-13T16:35:00.321Z] 16:35:00 INFO - log@resource://gre/modules/Log.jsm:360:7
[task 2019-01-13T16:35:00.323Z] 16:35:00 INFO - error@resource://gre/modules/Log.jsm:368:5
[task 2019-01-13T16:35:00.325Z] 16:35:00 INFO - _logMessage@resource://gre/modules/Extension.jsm:363:26
[task 2019-01-13T16:35:00.327Z] 16:35:00 INFO - logError@resource://gre/modules/Extension.jsm:359:5
[task 2019-01-13T16:35:00.330Z] 16:35:00 INFO - packagingError@resource://gre/modules/Extension.jsm:346:5
[task 2019-01-13T16:35:00.332Z] 16:35:00 INFO - manifestError@resource://gre/modules/Extension.jsm:332:5
[task 2019-01-13T16:35:00.336Z] 16:35:00 INFO - parseManifest@resource://gre/modules/Extension.jsm:601:7
[task 2019-01-13T16:35:00.338Z] 16:35:00 INFO - asyncloadManifest@resource://gre/modules/Extension.jsm:842:7
[task 2019-01-13T16:35:00.340Z] 16:35:00 INFO - async_receiveMessageAPI@chrome://mochikit/content/tests/SimpleTest/SpecialPowersObserverAPI.js:605:9
[task 2019-01-13T16:35:00.342Z] 16:35:00 INFO - ChromePowers.prototype._receiveMessage@chrome://mochikit/content/tests/SimpleTest/ChromePowers.js:81:14
[task 2019-01-13T16:35:00.344Z] 16:35:00 INFO - ChromePowers.prototype._sendAsyncMessage@chrome://mochikit/content/tests/SimpleTest/ChromePowers.js:43:3
[task 2019-01-13T16:35:00.345Z] 16:35:00 INFO - startup@chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:2067:9
[task 2019-01-13T16:35:00.347Z] 16:35:00 INFO - testDefaultDetails@chrome://mochitests/content/browser/browser/components/extensions/test/browser/test-oop-extensions/browser_ext_browserAction_pageAction_icon_permissions.js:102:24
[task 2019-01-13T16:35:00.349Z] 16:35:00 INFO - AsyncTester_execTest/<@chrome://mochikit/content/browser-test.js:1108:34
[task 2019-01-13T16:35:00.350Z] 16:35:00 INFO - asyncTester_execTest@chrome://mochikit/content/browser-test.js:1099:16
[task 2019-01-13T16:35:00.351Z] 16:35:00 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:997:9
[task 2019-01-13T16:35:00.353Z] 16:35:00 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59
[task 2019-01-13T16:35:00.354Z] 16:35:00 INFO -
[task 2019-01-13T16:35:00.356Z] 16:35:00 INFO - Console message: [JavaScript Error: "Security Error: Content at moz-extension://4175c09b-edf1-49ee-9930-f31a0169d5b5/ may not load or link to chrome://browser/content/browser.xul."]
[task 2019-01-13T16:35:00.358Z] 16:35:00 INFO - Console message: [JavaScript Error: "Security Error: Content at moz-extension://4175c09b-edf1-49ee-9930-f31a0169d5b5/ may not load or link to chrome://browser/content/browser.xul."]
Comment 14•5 years ago
|
||
Reporter | ||
Comment 15•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5accb58d92118cbf02c9e698917f7188b19bec73
Assignee | ||
Updated•5 years ago
|
Comment 16•5 years ago
|
||
Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7bf0d21448d
Fix Bug 1419109 - Empty default icon shows blank icon in toolbar r=mixedpuppy
Comment 17•5 years ago
|
||
bugherder |
Comment 18•5 years ago
|
||
Thanks for the patch, Varun! 🎉 Your contribution has been added to our recognition wiki: https://wiki.mozilla.org/Add-ons/Contribute/Recognition
Would you be interested in creating a profile on mozillians.org? I'd be happy to vouch for you!
Assignee | ||
Comment 19•5 years ago
|
||
Thanks Caitlin. My mozilillians profile is https://mozillians.org/en-US/u/varundey/ :)
Comment 20•5 years ago
|
||
You're vouched! Welcome onboard. I look forward to seeing you around!
Comment 21•5 years ago
|
||
Verified that the error mentioned in comment #10 is shown if there is is no value in the "default_icon". I've used the same test extension to test if it installs successfully and throws the error having only edited the default_icon value. Testing was run on Firefox 66.0b3, 65 and Nightly 67.0a1.
Description
•