Closed
Bug 1385864
Opened 7 years ago
Closed 7 years ago
Web extension with "chrome://favicon/" permissions in manifest breaks webrequest
Categories
(WebExtensions :: Request Handling, defect, P1)
Tracking
(firefox56 fixed, firefox57 fixed)
RESOLVED
FIXED
mozilla57
People
(Reporter: crownofdesign, Assigned: zombie)
References
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
kmag
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36 Steps to reproduce: I am using the following code for test: browser.webRequest.onBeforeSendHeaders.addListener( event => { console.log( 'onBeforeSendHeaders' ); console.log( event ); return; }, { 'urls': [ '<all_urls>' ] }, [ 'blocking', 'requestHeaders' ] ); This code in Embedded WebExtension, not pure webextension Actual results: console.log is not outputted in FF dev edition. In current normal Firefox all is working Expected results: It must work!
My manifest permissions: "permissions": [ "background", "proxy", "storage", "webRequest", "webRequestBlocking", "<all_urls>" ],
Updated•7 years ago
|
Component: Untriaged → WebExtensions: Request Handling
Product: Firefox → Toolkit
Updated•7 years ago
|
Version: 54 Branch → 55 Branch
Minimal code for testing: manifest.json { "manifest_version": 2, "name": "dev", "short_name": "dev", "version": "3.16.8", "description": "description", "background": { "scripts": [ "background.js" ] }, "icons": { "64": "app_icon64.png" }, "permissions": [ "background", "proxy", "storage", "webRequest", "webRequestBlocking", "<all_urls>" ], "optional_permissions": [ "management", "chrome://favicon/" ], "content_security_policy": "script-src 'self' https://ssl.google-analytics.com; object-src 'self'" }
background.js console.log( 'background start' ); browser.webRequest.onBeforeSendHeaders.addListener( event => { console.log( 'onBeforeSendHeaders' ); console.log( event ); return; }, { 'urls': [ '<all_urls>' ] }, [ 'blocking', 'requestHeaders' ] ); console.log( 'background test 1' );
By chat conservations: in just FF beta there is no bug, in FF dev edition there is bug
Personally tested in pure Firefox Beta - this test (above) does not work for me
Summary: webextension: browser.webRequest.onBeforeSendHeaders is not working in current firefox dev edition → webextension: browser.webRequest.onBeforeSendHeaders is not working in current firefox beta
Assignee | ||
Comment 8•7 years ago
|
||
The issue is with your manifest, firefox doesn't support the "chrome://favicon/" permission, so your extension doesn't even load. Unfortunately, the error is invisible unless you are using the current Nightly (which is generally suggested while developing).
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 10•7 years ago
|
||
What it's working in FF54?
Reporter | ||
Comment 11•7 years ago
|
||
*Why
Assignee | ||
Comment 12•7 years ago
|
||
Not sure, we probably started being strict about permissions in 55.
Summary: webextension: browser.webRequest.onBeforeSendHeaders is not working in current firefox beta → Web extension with unknown permissions in manifest fails to load
Comment 13•7 years ago
|
||
Sound like bug 1373293 which was fixed in 56.
Resolution: INVALID → DUPLICATE
Assignee | ||
Comment 14•7 years ago
|
||
But that says it got uplifted to 55, which doesn't match my testing. Reopening since it seems the fix for bug 1373293 missed optional_permissions, and this is breaking webrequests as is.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Summary: Web extension with unknown permissions in manifest fails to load → Web extension with "chrome://favicon/" permissions in manifest breaks webrequest
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → tomica
Status: REOPENED → ASSIGNED
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8893597 [details] Bug 1385864 - Drop invalid optional_permissions from manifests https://reviewboard.mozilla.org/r/164684/#review170098
Attachment #8893597 -
Flags: review?(kmaglione+bmo) → review+
Comment 17•7 years ago
|
||
Pushed by tomica@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ca0419d7845f Drop invalid optional_permissions from manifests r=kmag
Assignee | ||
Comment 18•7 years ago
|
||
Comment on attachment 8893597 [details] Bug 1385864 - Drop invalid optional_permissions from manifests Approval Request Comment [Feature/Bug causing the regression]: Bug 1322235 [User impact if declined]: This prevents certain extensions from being installed. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: No. [Needs manual test from QE? If yes, steps to reproduce]: No, manually tested from local build. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: Low-risk. [Why is the change risky/not risky?]: it's very simple, just marks unknown strings to be dropped from manifest. [String changes made/needed]: None.
Attachment #8893597 -
Flags: approval-mozilla-beta?
Comment 19•7 years ago
|
||
Comment on attachment 8893597 [details] Bug 1385864 - Drop invalid optional_permissions from manifests Looks like we need this for webextensions to work - let's uplift for 56 beta 1.
Attachment #8893597 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 20•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ca0419d7845f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 21•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/739ff1fa9eaf
status-firefox56:
--- → fixed
Flags: in-testsuite+
Comment 22•7 years ago
|
||
(In reply to Tomislav Jovanovic :zombie from comment #18) > [Is this code covered by automated tests?]: Yes. > [Has the fix been verified in Nightly?]: No. > [Needs manual test from QE? If yes, steps to reproduce]: No, manually tested > from local build. Setting qe-verify- based on Tomislav's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Comment 23•7 years ago
|
||
Related to bug 1315616?
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•