Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebExtension API #65

Closed
tshaddix opened this issue Feb 20, 2017 · 41 comments
Closed

WebExtension API #65

tshaddix opened this issue Feb 20, 2017 · 41 comments

Comments

@tshaddix
Copy link
Owner

We had a great discussion going on in #60 regarding first-class support for WebExtensions and other browsers.

@vhmth @AgrimPrasad @paulius005 Let's continue the discussion.

@tshaddix
Copy link
Owner Author

The WebExtension API looks pretty solid, especially with the cross-browser support. I would be down to adopt the API if we feel confident in it.

@vhmth
Copy link
Collaborator

vhmth commented Feb 20, 2017

These are the things I propose we do for this migration (if everyone is onboard - I'm 100% game):

  1. Convert all chrome.* calls to browser.* calls.
  2. Include a polyfill build and a non-polyfill build (leave it up to the user to include their own polyfill).
  3. Document the browser.* calls we make in the wiki.

@paulius005
Copy link
Contributor

@vhmth totally down to help with this effort.

The conversation in #61 about onMessageExternal makes me wonder what browser specific calls we should have baked in since it is not supported by FF: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onMessageExternal

@vhmth
Copy link
Collaborator

vhmth commented Feb 20, 2017

chrome.* APIs and where they're used:

Obviously all of these are supported by Chrome. Here's how it's supported by other browsers. Format:

feature - <FF> | <Edge>

  • chrome.runtime.connect - Yes | No
  • chrome.runtime.sendMessage - Yes | Yes
  • chrome.runtime.onMessage - Yes (45.0+) | Yes
  • chrome.runtime.onMessageExternal - No | No
  • chrome.runtime.onConnect - Yes (45.0+) | No
  • chrome.runtime.onConnectExternal - No | No

It seems like external messaging support is non-existent for FF and Edge, so we should probably make a note about that in the README depending on what happens to the discussion in #61

At first I was surprised that onConnect is not supported in IE, but then I realized that the Edge Extension API is largely ongoing. I read that as, we shouldn't worry about it for now.

@tshaddix
Copy link
Owner Author

tshaddix commented Mar 1, 2017

@vhmth This is awesome. Thanks for going through that list.

I'm happy to continue support of external messaging in chrome, but not allow it to be a blocker in supporting the other extensions. I think the usage of external messaging is relatively low, currently.

@vhmth
Copy link
Collaborator

vhmth commented Mar 4, 2017

For sure! I think we will end up using external messaging very soon. We have something cool being worked on that may require it. :-)

@tshaddix
Copy link
Owner Author

tshaddix commented Mar 5, 2017

@vhmth Put me on that alpha list 🥇

@vhmth
Copy link
Collaborator

vhmth commented Mar 6, 2017

You already know. :-) There actually won't be any alpha/beta testing. It'll roll out to public at once. I'll show you where it's being used when it goes out!

@Techdojo
Copy link

Techdojo commented Mar 8, 2017

I'm really interested in seeing this library being ported so that it's usable across Chrome, Firefox, Safari, IE (10+) and Edge - still a bit new to git and distributed collaborative development, but I'm happy to help out as much as I can.

Personally I see this method of using react and redux as the ultimate in cross browser extension development - just my $0.02 :)

@vhmth
Copy link
Collaborator

vhmth commented Mar 8, 2017

I couldn't agree more @Techdojo. We are very happy to have you here in our tiny corner of GitHub! Welcome!

@collinsauve
Copy link
Contributor

Regarding using a polyfil build: Might I suggest https://github.com/mozilla/webextension-polyfill?

Also how about directly using the browser API without a polyfill, and suggesting that Chrome users include their own copy of the above polyfill? This library itself then wouldn't have to decide to polyfill or not.

@tshaddix
Copy link
Owner Author

@collinsauve Great call with the polyfill!

Definitely down to have that discussion. I think as long as we make it uber clear how to use the polyfill with chrome we can consider that an option.

We definitely don't want to make chrome support second class :)

@vhmth
Copy link
Collaborator

vhmth commented Mar 13, 2017

I think creating a polyfill and non-polyfill build is the way to go here still. Then we could include some documentation detailing that, if you're building a Chrome extension, you will definitely want to include the build with the polyfill. Otherwise, if you're only building a FF or IE Edge extension, you can use the build without the polyfill.

@collinsauve
Copy link
Contributor

collinsauve commented May 25, 2017

Since I already have some experience converting a Chrome extension to use WebExtensions, I thought I'd take a stab at this tonight.

Probably the only somewhat-difficult part of the conversion is converting callback-based functions to promise-based. Specifically onMessage.addListener function is problematic. Do NOT trust the MDN documentation or even the Browser Extensions draft spec for this - there are plans to remove the sendResponse callback. I am still not clear on the exact behavior that will they will land on for these APIs, but I digress...

Anyways, I have a question for you guys: I kind of got stuck on trying to understand what this dispatchResponder is for. I'm also not clear why you use a promise here instead of just a try/catch. Could someone help me understand? I've opened #82 if you want to take the explanation there (and maybe document it in readme? :) )

@tshaddix
Copy link
Owner Author

Hey @collinsauve !

Glad to hear you're going to take this on! Thanks for doing the research and recording it back here.

Have you seen this: https://github.com/tshaddix/react-chrome-redux/wiki/Advanced-Usage#dispatchresponder ?

@collinsauve
Copy link
Contributor

collinsauve commented May 25, 2017

Edit: Note that this commit definitely has some problems that I have encountered but not had a chance to properly diagnose/fix. I should be circling back to this in the next couple of weeks, but please do not try to use this in any production builds as of yet.

See collinsauve@5f6975b for main code changes to switch to WebExtensions. So far I have lightly tested this with my project and the store seems to be synchronizing properly (and aliases running FWIW). I am not using some of the more advanced features of this library, so I'm worried I might have broken something. Any help testing would be appreciated (esp. the expected behaviour of dispatchResponder and onMessageExternal/onConnectExternal).

The above commit assumes that browser is in global scope. Depending on how one is using the polyfill this may not be the case. See collinsauve@37bc22c for importing webextension-polyfill as a npm module. Unfortunately I couldn't get mozilla/webextension-polyfill to work as an npm module (it doesn't have an install step) so I just created a branch with it pre-compiled. Obviously looking for better solutions here, as well as help setting up "a polyfill and non-polyfill build".

@jdinartejesus
Copy link

Hello @collinsauve and @tshaddix!

The missing API's from FF are now available on the version +54, it's still part of the plan support other browsers?

@Techdojo
Copy link

Sorry I've been away from this discussion for a while (something about real life and paying work but I digress ;)) anyway just wondering what the current state of play is with this work it seems to have gone quiet?

Anything I can do to help? I'm particularly interested in getting this library to run on FireFox if possible.

@tshaddix
Copy link
Owner Author

@jdinartejesus Hello hello! Let's do it!

@Techdojo I'm in the same boat! Had a lot of stuff going on in the background but I'm happy to spend some time pushing forward on this.

@vhmth @paulius005 You guys interested in making this happen?

@vhmth
Copy link
Collaborator

vhmth commented Aug 20, 2017

@paulius005 is currently out on vacation. I'd say that this is definitely a larger, systemic, one-man job (rather than trying to split up the work between peeps).

@Techdojo if you're willing to push forward on this, I'd be happy to be super on top of code reviews and getting back to you very quickly. I can also line up a Saturday to be generally available while you crunch through. Right now my company is going through some pretty wild times, so it's hard to promise any work here.

@Techdojo
Copy link

Ok I'll post back my findings - I'm a bit rusty on contributing to open source projects so please bear with me, but I'm sure I can sort something out.

@vhmth
Copy link
Collaborator

vhmth commented Aug 22, 2017

Thank you @Techdojo. It's ok - let us know if you're having any problems figuring out how to contribute back. I can walk you through the "fork -> PR" flow if you'd like.

@Techdojo
Copy link

Sorry it's been a while since I last posted, I'm back working on this project again after successfully releasing a firefox build - one thing I 'discovered' by accident is that for the time being the latest version of Firefox (as well as the Nov 17 - v57 update that will disable all none webextension format extensions) actually work with this library "out of the box" and no addition is required to get a webextension powered firefox version to work.

Firefox actually supports both the promise based WebExtension api as well as the callback based Chrome api.

From what I've read Edge supports a callback version of the WebExtension api and will require some detection and updating where as our old friend Safari is going to need the most amount of work (I'm just starting work on a Safari wrapper for this library).

Also there is a "new / preferred" method that Apple require to get your extension into the App Store which is to basically rewrite it as a native MacOs app (the background page is replaced by a native ObjC / Swift app and the popup is replaced by Apple's UIKit). Although for now old style classic safari extensions are supported and I think you can still submit them to Apple's extension gallery.

It's probably best to create a new issue to track progress on implementing the Safari API, so I'll go ahead an open one.

@vhmth
Copy link
Collaborator

vhmth commented Nov 2, 2017

@Techdojo fantastic analysis! I was actually talking to one of the lead extension developers at Grammarly, and he had mentioned that Safari was by far the worst extension platform to develop on for many reasons. I don't think we'll be missing out on much by not fully trying to support it and tracking progress in a separate issue. Good call!

It's great to hear that things just work out of the box with Firefox!

@Techdojo
Copy link

Techdojo commented Nov 2, 2017

@vhmth with respect to Safari, I started along the path by creating a polyfill / shim for the chrome.* functions which was as far as I managed to get before my PO decided that Safari was going to take too long as there are massive differences in the way that core extension functionality is implemented (especially if we're planning on going down the native app route later anyway) and was tasked with creating the Edge version instead.

My first point of call was the M$ Edge Extension Toolkit that is supposed to automatically convert Chrome extensions for you - surprise surprise it didn't work (although to be fair I was giving in transpiled output). However I followed their basic approach and created a chrome.* polyfill that I run before importing the library and that actually seems to work. I do have an issue with browser.cookies.* not working (not sure if that's me or Edge that's at fault), but at least my redux dispatch messages are getting through and state is being returned.

I don't know the best way of implementing this within the framework of this library but I'm happy to share my polyfill with anyone who has a better idea.

@vhmth
Copy link
Collaborator

vhmth commented Nov 2, 2017

@Techdojo I think even just sharing here for historical reasons (others who stumble upon it) would be a huge help to others! We can then reference this issue too in the code if we implement it.

@Techdojo
Copy link

Techdojo commented Nov 2, 2017

Ok, I'll get the code cleaned up and posted here as soon as I can.

@vhmth
Copy link
Collaborator

vhmth commented Nov 2, 2017

🎉

@jdinartejesus
Copy link

Hello everyone, there is any updated on this issues?

@vhmth
Copy link
Collaborator

vhmth commented Jan 15, 2018

@Techdojo I know it's been a while but did you want to drop this officially? We could make a tag for Need help or something so others can jump in.

@AllenFang
Copy link

@vhmth I think I can help on it, I really love this product and hope it can work with WebExtensions.

@vhmth
Copy link
Collaborator

vhmth commented Feb 15, 2018

Awesome @AllenFang! Looking forward to any work. We could really use the help. 😅

@DJWassink
Copy link
Contributor

DJWassink commented Jun 7, 2018

So going by the list @vhmth posted, here is the actual status (renamed to browser):

feature - FF | Edge

browser.runtime.connect - 45 | 14
browser.runtime.sendMessage - 45 | 14*
browser.runtime.onMessage - 45 | 14
browser.runtime.onMessageExternal - 45 | No
browser.runtime.onConnect - 45 | 14
browser.runtime.onConnectExternal - 45 | No

  • runtime.onMessage listeners in extension views receive the messages they sent.

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Browser_support_for_JavaScript_APIs

I have renamed every chrome to browser in this repo and ran it in firefox, no problems there. I am planning on porting my complete extension to support edge also (firefox already works with this lib and other chrome.runtime api's).

So if I am correct the external things won't work in edge, but the external is only used if a script is manually injected in a page right? Not if it is injected through the manifest? If so, isn't simply renaming the chrome.runtime things to browser.runtime enough? Maybe a little warning in the readme for people who depending on this behavior and tell people to include a polyfill for chrome browsers.

@vhmth
Copy link
Collaborator

vhmth commented Jun 10, 2018

Woah this is HUGE news @DJWassink! Can't believe just changing all chrome. to browser. worked for you! Perhaps we could just make this easy and introduce a shim util?

// runtime.js util
export const connect => (window.chrome || window.browser).runtime.connect
export const sendMessage => (window.chrome || window.browser).runtime.sendMessage
...

Like @DJWassink said, a simple warning/feature map in the README should be enough. Thoughts @tshaddix?

@DJWassink
Copy link
Contributor

DJWassink commented Jun 10, 2018

To be honest, after some more thought about this I am a bit sceptical about my findings. Simply changing every chrome to browser shouldn't be a solution since the chrome api should work with callbacks and the browser with promises, maybe firefox its solution accepts callbacks in both and figures out that way if you use the chrome way or the WebExtensions way (since FF implements both).

However rewriteing some parts to use the browser/WebExtension variant and letting the users include the polyfill sounds like the best solution for me. Or we have to manually make promise wrappers around the used chrome calls and make our own small polyfill

@vhmth
Copy link
Collaborator

vhmth commented Jun 10, 2018

I see. We incur technical debt by supporting multiple browsers, but I feel like making a promise wrapper against the small subset of browser APIs we use isn't crazy (personally). I'm not super passionate either way, but just my 2 bitcoin.

@DJWassink
Copy link
Contributor

Yeah I feel the same, but honestly having a separate shim around the different api's sounds like the correct solution :) sadly we have put support for other browsers then firefox and chrome back in the freezer, but in the future I will definitely come back to this!

@tshaddix
Copy link
Owner Author

Hey all, beginning to revisit this @vhmth @DJWassink .

I think we may consider just doing the quick solution @vhmth suggested for now, which is building out a basic utility that uses global.chrome or global.browser based on availability.

From the Mozilla documentation, it looks like they are very intentionally supporting callbacks for now to support chrome extensions:

Note that this is different from Google Chrome's extension system, which uses the chrome namespace instead of browser, and which uses callbacks instead of promises for asynchronous functions. As a porting aid, the Firefox implementation of WebExtensions APIs supports chrome and callbacks as well as browser and promises.

And when it comes to MSEdge support, it looks like even the polyfill team is taking a breather:

With the Microsoft announcement related to the MSEdge future there aren't many compelling reasons to officially include in this polyfill any workarounds specific to the current MSEdge versions (and then maintaining them over the time), and so MSEdge is currently unsupported. Once the first Chrome-based MSEdge version is going to be released, we will be able to verify if any actual changes to this polyfill are needed to officially support the Chrome-based MSEdge versions.

Because of this, I think we can move forward with some short-term confidence that a quick and easy enhancement is including this utility. Looking forward to your thoughts :)

This was referenced Feb 21, 2019
@DJWassink
Copy link
Contributor

Good point, now that MS is ditching Edge there are no more hurdles, FF even supports all the currently used API's since FF 54.

Just for a little headsup, I am already using this package in a FF/Chrome extension without any problems!

@tshaddix
Copy link
Owner Author

@DJWassink Great! I figured you would be able to use it on FF because it has an automatic shim for the chrome global. I'll go ahead and wait a little longer for disagreements before pushing #185 forward.

@tshaddix
Copy link
Owner Author

Released in the 2.0.0 of webext-redux :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants