Closed
Bug 1237822
Opened 8 years ago
Closed 8 years ago
"tabs" permission is needed to query by URL
Categories
(WebExtensions :: Untriaged, defect, P4)
WebExtensions
Untriaged
Tracking
(firefox48 verified, firefox52 verified)
VERIFIED
FIXED
mozilla48
People
(Reporter: wbamberg, Assigned: tofumatt)
Details
(Whiteboard: [permissions][berlin][good first bug] triaged)
Attachments
(2 files)
I've attached an extension that adds a browser action: when you click the browser action the extension runs code like this: function query() { chrome.tabs.query({'url': "http://www.bbc.com/"}, (tabs) => {console.log(tabs.length);}); } This fails, because in the code here: https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/ext-tabs.js#431, tab.url is undefined. If I give the extension the "tabs" permission, it works. The same extension works in Chrome without needing the "tabs" permission.
Updated•8 years ago
|
Flags: blocking-webextensions?
Updated•8 years ago
|
Whiteboard: [permissions]
Comment 1•8 years ago
|
||
Honestly, I think our behavior is closer to correct. It definitely shouldn't throw, but if you're not meant to be able to access tab URLs, you also shouldn't be able to do things that let you probe for them.
Reporter | ||
Comment 2•8 years ago
|
||
I agree with you, I think, but we should be clear that correctness is worth introducing the incompatibility. Anyway, I've noted this in https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities and https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/permissions.
Updated•8 years ago
|
Priority: -- → P4
Whiteboard: [permissions] → [permissions] triaged
Updated•8 years ago
|
Flags: blocking-webextensions? → blocking-webextensions+
Updated•8 years ago
|
Whiteboard: [permissions] triaged → [permissions][berlin] triaged
Updated•8 years ago
|
Whiteboard: [permissions][berlin] triaged → [permissions][berlin][good first bug] triaged
Comment 3•8 years ago
|
||
Do we agree to WONTFIX this?
Comment 4•8 years ago
|
||
No, we still need to handle it properly. We should probably just throw if an extension without "tabs" permissions tries to query based on these properties.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → tofumatt
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40129/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40129/
Attachment #8730735 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8730735 [details] MozReview Request: bug 1237822: Throw error if tabs.query is used without "tabs" permission with url param r?kmag Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40129/diff/1-2/
Comment 7•8 years ago
|
||
Comment on attachment 8730735 [details] MozReview Request: bug 1237822: Throw error if tabs.query is used without "tabs" permission with url param r?kmag https://reviewboard.mozilla.org/r/40129/#review36657 Thanks ::: browser/components/extensions/ext-tabs.js:557 (Diff revision 2) > > query: function(queryInfo) { > let pattern = null; > if (queryInfo.url !== null) { > + if (!extension.hasPermission("tabs")) { > + return Promise.reject({message: 'The "tabs" permission is required to use the query API'}); It's only required to query by URL.
Attachment #8730735 -
Flags: review?(kmaglione+bmo) → review+
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8730735 [details] MozReview Request: bug 1237822: Throw error if tabs.query is used without "tabs" permission with url param r?kmag Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40129/diff/2-3/
Attachment #8730735 -
Attachment description: MozReview Request: bug 1237822: Throw error if tabs.query is used without "tabs" permission r?kmag → MozReview Request: bug 1237822: Throw error if tabs.query is used without "tabs" permission with url param r?kmag
Assignee | ||
Comment 9•8 years ago
|
||
https://reviewboard.mozilla.org/r/40129/#review36657 > It's only required to query by URL. Thanks, updated the error message to explicitly mention the "url" param.
Assignee | ||
Comment 10•8 years ago
|
||
Tests seem good, how do I get this merged?
Comment 11•8 years ago
|
||
Just add the checkin-needed keyword.
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/76d23219d38e
Keywords: checkin-needed
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/76d23219d38e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 14•8 years ago
|
||
I could reproduce this issue on Firefox 48.0a1 (20160321030217), here is a video: https://www.dropbox.com/s/xyz7liol0d6f4cw/error1.gif?dl=0 This issues is verified as fixed on Firefox 48.0a1 (20160324154036)and Firefox 52.0a1 (20160922030437) under Windows 7 64-bit, the error message: “ 'The "tabs" permission is required to use the query API with the "url" parameter “ is displayed in the browser console, here is a video: https://www.dropbox.com/s/av3r5rf7fs1uvj1/error2.gif?dl=0
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•