Opened 3 years ago

Last modified 20 months ago

#4579 closed change

Target promise-based browserext API — at Version 16

Reported by: kzar Assignee: mjethani
Priority: P3 Milestone: Adblock-Plus-3.0.3-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: sebastian, oleksandr, greiner Blocked By: #4551, #4783, #5028
Blocking: #4580 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29573892/
https://codereview.adblockplus.org/29678657/

Description (last modified by kzar)

Background

We want to target the browserext standard instead of the Chrome extension API. This should hopefully mean Edge and in the future Gecko can be supported without pollyfills.

What to change

  • Add a polyfill for the Chrome extension to add browserext compatibility, adding the browser namespace, wrapping asynchronous APIs to use promises and mapping the others directly. (See this existing polyfill
  • Replace any other use of the Chrome APIs with the with the browserext API.

Change History (16)

comment:1 Changed 3 years ago by kzar

  • Blocked By 4551 added; 4511 removed

comment:2 Changed 3 years ago by kzar

  • Description modified (diff)

comment:3 Changed 3 years ago by kzar

  • Description modified (diff)
  • Summary changed from Remove ext compatibility layer, target browser extension API to Target the browser extension API

comment:4 Changed 3 years ago by sebastian

  • Priority changed from Unknown to P3
  • Ready set

comment:5 Changed 3 years ago by kzar

  • Blocking 4580 added

comment:6 Changed 3 years ago by kzar

  • Summary changed from Target the browser extension API to Target the browserext API

comment:7 Changed 3 years ago by kzar

  • Cc oleksandr added
  • Owner set to kzar

comment:8 Changed 3 years ago by kzar

  • Blocked By 4783 added

comment:9 Changed 3 years ago by greiner

  • Cc greiner added

comment:10 Changed 3 years ago by sebastian

Giving it a second though (and having a go myself) it seems too early to target (and polyfill) the Promise based API. As of now, neither Chrome nor Microsoft Edge supports Promises with their API. Only Firefox does, which also optionally accepts callbacks. As a result callbacks (as opposed to Promises) are the common denominator of all targeted browsers. Implementing a polyfill that allows us to use Promises will add complexity and overhead.

But what I think we should already do is replacing all occurrences of chrome with browser, aliasing browser = chrome on Chrome. We already did it the other way around in the edge branch (#3695), and we might need it for Firefox/WebExtensions as well. However, it feels backwards, to keep using the chrome.* namespace, while this code runs on other browser, and browser is going to be standardized.

comment:11 Changed 3 years ago by kzar

I already made a lot of progress towards targeting the browser API, if you're curious to have a look here's my branch. It has been a while since I looked at it but IIRC I didn't quite finish, and also I planned to try using a lighter weight polyfill that Wladimir suggested. (Also I need to rebase again...) I did have mostly everything working however.

But I kind of agree with you, I think it might well be a little early to make the switch.

comment:12 Changed 3 years ago by kzar

  • Owner kzar deleted

Unassigned myself since I don't have the time to continue working on this currently and as per the above discussion we're not sure if it's the right time to make the switch anyway.

comment:13 Changed 3 years ago by sebastian

  • Blocked By 5028 added

comment:14 Changed 3 years ago by sebastian

  • Ready unset

I moved the part about using browser.* with callbacks (instead of promises), for now, into #5028.

comment:15 Changed 3 years ago by sebastian

  • Summary changed from Target the browserext API to Target promise-based browserext API

comment:16 Changed 2 years ago by mjethani

  • Owner set to mjethani
  • Review URL(s) modified (diff)
Note: See TracTickets for help on using tickets.