Opened 3 years ago

Closed 2 years ago

#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/
https://codereview.adblockplus.org/29573083/
https://codereview.adblockplus.org/29573134/

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.*.

Change History (19)

comment:1 Changed 3 years ago by sebastian

  • Description modified (diff)

comment:2 Changed 3 years ago by greiner

  • Cc greiner added

comment:3 Changed 2 years ago by mjethani

Mozilla has a polyfill for this.

comment:4 Changed 2 years ago by sebastian

  • Description modified (diff)

comment:5 Changed 2 years ago 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 2 years ago by sebastian

  • Priority changed from P3 to P4

comment:7 Changed 2 years ago by sebastian

  • Description modified (diff)

comment:8 follow-up: Changed 2 years ago 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 2 years ago 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 2 years ago 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 2 years ago by mjethani

  • Owner set to mjethani

comment:12 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:13 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)

comment:14 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5028 - Use browser namespace

comment:15 Changed 2 years ago by kzar

  • Blocking 5080 added

comment:16 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5028 - Use browser namespace

comment:17 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5028 - Add missing polyfill.js

comment:18 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5028 - Use browser namespace

comment:19 Changed 2 years ago by mjethani

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.