Opened on 03/22/2017 at 09:11:37 AM
Closed on 10/17/2017 at 01:10:36 PM
#5028 closed change (fixed)
Use the browser extension API via the "browser" (instead of "chrome") namespace
| Reported by: | sebastian | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P4 | Milestone: | Adblock-Plus-3.0-for-Firefox |
| Module: | Platform | Keywords: | goodfirstbug |
| Cc: | greiner, mjethani | Blocked By: | |
| Blocking: | #4579, #5080 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): |
https://codereview.adblockplus.org/29570614/ |
||
Description (last modified by sebastian)
Background
In the upcoming browserext standard, which is already implemented by Firefox and Microsoft Edge, the browser extension API is provided by the browser namespace, as opposed to the chrome namespace.
As a quick fix, we therefore aliased chrome = browser in the edge branch (#3695), in Firefox we can either use chrome or browser. However, given that browser.* is going to be standardized, this is backwards, and using chrome.* in code that also runs on other platforms is also misleading.
What to change
- If the browser object is undefined (i.e. on Chrome), globally define browser = chrome.
- Adapt all usage of the extension API to use browser.* instead of chrome.*.
Attachments (0)
Change History (19)
comment:2 Changed on 04/03/2017 at 12:24:36 PM by greiner
- Cc greiner added
comment:3 Changed on 05/14/2017 at 02:39:56 AM by mjethani
comment:5 Changed on 06/13/2017 at 10:55:29 AM by sebastian
- Cc mjethani added
That polyfill is rather something to address #4579. Moreover, that particular polyfill is incompatible with Microsoft Edge as it assumes that the API supposrts promises when it is exposed as browser.
comment:6 Changed on 06/13/2017 at 10:57:11 AM by sebastian
- Priority changed from P3 to P4
comment:8 follow-up: ↓ 10 Changed on 08/25/2017 at 07:48:20 AM by mjethani
If we're going to use browser.* and still use it with callbacks, doesn't this mean we'll have to reassign browser to chrome on Firefox as well?
But then the original browser.* will become unreachable. There is at least one instance of it being used directly, though this may not be necessary.
comment:9 Changed on 10/02/2017 at 05:00:57 PM by mjethani
I think we should try to use the promise-based API and do some magic on Chrome and Edge to make it work that way.
comment:10 in reply to: ↑ 8 Changed on 10/02/2017 at 05:22:25 PM by sebastian
Replying to mjethani:
If we're going to use browser.* and still use it with callbacks, doesn't this mean we'll have to reassign browser to chrome on Firefox as well?
We don't. On Firefox you can use either the chrome or browser namespace with either callbacks or promises. In both cases a promise is returned if no callback is given.
Replying to mjethani:
I think we should try to use the promise-based API and do some magic on Chrome and Edge to make it work that way.
Using the promise-based API, with a polyfill, is being discussed in #4579. Regardless, what we decide there, it seems to make sense to use the API under it's standard name.
comment:11 Changed on 10/09/2017 at 03:11:08 PM by mjethani
- Owner set to mjethani
comment:12 Changed on 10/09/2017 at 05:06:14 PM by mjethani
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:13 Changed on 10/10/2017 at 10:56:12 PM by mjethani
- Review URL(s) modified (diff)
comment:14 Changed on 10/13/2017 at 08:35:02 PM by abpbot
A commit referencing this issue has landed:
Issue 5028 - Use browser namespace
comment:15 Changed on 10/15/2017 at 01:31:02 PM by kzar
- Blocking 5080 added
comment:16 Changed on 10/17/2017 at 12:49:07 PM by abpbot
A commit referencing this issue has landed:
Issue 5028 - Use browser namespace
comment:17 Changed on 10/17/2017 at 12:57:44 PM by abpbot
A commit referencing this issue has landed:
Issue 5028 - Add missing polyfill.js
comment:18 Changed on 10/17/2017 at 01:07:54 PM by abpbot
A commit referencing this issue has landed:
Issue 5028 - Use browser namespace
comment:19 Changed on 10/17/2017 at 01:10:36 PM by mjethani
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
Mozilla has a polyfill for this.