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

Will this project get support for Safari? #234

Closed
149segolte opened this issue Jun 27, 2020 · 20 comments
Closed

Will this project get support for Safari? #234

149segolte opened this issue Jun 27, 2020 · 20 comments

Comments

@149segolte
Copy link

149segolte commented Jun 27, 2020

In a WWDC session, Apple announced support for Web extensions on macOS and has provided conversion tools to port extensions.
The official documentation is at the link: https://developer.apple.com/documentation/safariservices/safari_web_extensions

@Poopooracoocoo
Copy link

it's only for macOS and not iOS fyi

@149segolte
Copy link
Author

If the company is making efforts to make this available on macOS then it might lead to it later being ported to iOS.

@Juraj-Masiar
Copy link

Juraj-Masiar commented Oct 1, 2020

I've been looking into porting my addons to Safari but there is one crucial feature missing - the runtime.onMessage API in there doesn't support replying to the message by returning a Promise, only by calling the sendResponse function which I don't use because it's not recommended and it doesn't exists in this polyfill anymore.

Would implementing this feature for Safari be outside of scope for this polyfill?

@rpl
Copy link
Member

rpl commented Oct 1, 2020

I've been looking into porting my addons to Safari but there is one crucial feature missing - the runtime.onMessage API in there doesn't support replying to the message by returning a Promise, only by calling the sendResponse function which I don't use because it's not recommended and it doesn't exists in this polyfill anymore.

Would implementing this feature for Safari be outside of scope for this polyfill?

I didn't spent yet enough time on tracking down all the differences in the Safari implementation to answer that question in a way as complete and final as I would like, but the following are some initial thoughts:

  • Given that Safari should be already providing a Promised API, most of the code in this polyfill isn't really going to be that much useful on Safari, the runtime.onMessage API that @Juraj-Masiar did mention may be the only piece that may be kind of reasonable to polyfill.

  • Safari extension contexts do have both chrome and browser globals, they should be basically aliases and the asynchronous API methods in both of them to be returning a Promise when a callback is not passed (while when there is a callback no promise is returned), and so at the moment this polyfill is likely turning itself into a noop as it does on Firefox, because the browser globals is already defined.

  • As an additional side note: to officially support another browser, especially for a separate implementation like in this case, the integration tests in this repo should also run in that browser as part of the CI (which is another detail that I haven't digged into yet), it wouldn't be really "supported" without automated tests running on the CI.

About the support for the sendResponse, it does print a deprecation warning the first time that it is being used but we didn't remove it from this polyfill:

let sendResponsePromise = new Promise(resolve => {
wrappedSendResponse = function(response) {
if (!loggedSendResponseDeprecationWarning) {
console.warn(SEND_RESPONSE_DEPRECATION_WARNING, new Error().stack);
loggedSendResponseDeprecationWarning = true;
}
didCallSendResponse = true;
resolve(response);
};
});

And so as a short term workaround, sendResponse should work fine across the 3 implementation (Firefox, Safari and Chrome).

@Juraj-Masiar
Copy link

Juraj-Masiar commented Oct 1, 2020

Thanks @rpl for the info!
I have a huge code base so there is no way I'm gonna refactor it from superior Promises to callbacks :)

But I guess I could write my own simple polyfill, something like this should work:

// if (IS_SAFARI) do this:
browser.runtime.onMessage.addListener = new Proxy(browser.runtime.onMessage.addListener, {
  apply: (addListener, thisArg, [handler]) => addListener.call(thisArg, (message, sender, sendResponse) => {
    const result = handler(message, sender, sendResponse);
    if (result instanceof Promise) {
      result.then(sendResponse);
      return true;
    }
    if (result === true) return true;
  })
});

EDIT: I've just tested it and it works. But I've edit it a bit to still support sendResponse function because there is some internal safari code that uses it and it didn't liked my exception :)

@bcomnes
Copy link

bcomnes commented Jun 1, 2021

I've been looking into porting my addons to Safari but there is one crucial feature missing - the runtime.onMessage API in there doesn't support replying to the message by returning a Promise, only by calling the sendResponse function which I don't use because it's not recommended and it doesn't exists in this polyfill anymore.

Would implementing this feature for Safari be outside of scope for this polyfill?

I ran into this issue this weekend. Also, I noticed that safari did not fire events for browser.storage.onChanged.addListener when you try to browser.storage.local.set a storage key to a falsey value like null. You have to explicitly browser.storage.local.remove the key to trigger that event.

@ivanjeremic
Copy link

Wonder why there is no cross-browser extension framework yet on the market.

@Rob--W
Copy link
Member

Rob--W commented Jun 22, 2021

Wonder why there is no cross-browser extension framework yet on the market.

Years ago there was no cross-browser compatible API. These days we do, and the compatibility is going to improve even further by the recent launch of the WebExtensions Community Group - https://www.w3.org/community/webextensions/2021/06/04/forming-the-wecg/

@gangsthub
Copy link

Wonder why there is no cross-browser extension framework yet on the market.

+1000 on that.

I went to https://github.com/fregante/Awesome-WebExtensions hoping to find something new... and nope.

At least Chrome has now shiny new docs on Extensions: https://developer.chrome.com/docs/extensions/

@csandman
Copy link

csandman commented Oct 7, 2021

Thanks @rpl for the info! I have a huge code base so there is no way I'm gonna refactor it from superior Promises to callbacks :)

But I guess I could write my own simple polyfill, something like this should work:

// if (IS_SAFARI) do this:
browser.runtime.onMessage.addListener = new Proxy(browser.runtime.onMessage.addListener, {
  apply: (addListener, thisArg, [handler]) => addListener.call(thisArg, (message, sender, sendResponse) => {
    const result = handler(message, sender, sendResponse);
    if (result instanceof Promise) {
      result.then(sendResponse);
      return true;
    }
    if (result === true) return true;
  })
});

EDIT: I've just tested it and it works. But I've edit it a bit to still support sendResponse function because there is some internal safari code that uses it and it didn't liked my exception :)

Do you have a consistent way of checking whether or not it is running in Safari in the code, for browser_action popups and background scripts?

@Juraj-Masiar
Copy link

Nope, but you can probably parse the value from the userAgent string.
I'm using Webpack to create separate package for each store (Firefox / Chrome / Safari / Thunderbird) and then webpack.DefinePlugin will simply replace these global placeholders. Although it's great that the placeholders are replaced before minification so the dead code is then dropped, it's terrible when you try to integrate existing code into different project that doesn't use Webpack. Also testing is problematic. I would probably used some different method if I turn back time :)

@csandman
Copy link

csandman commented Oct 7, 2021

@Juraj-Masiar I also have a setup which is using webpack to make a build for each browser, any chance you could elaborate on how you added it using that? I'm using webextension-toolbox and I tried to write a custom webpack script for the safari build using it but couldn't quite get it working right.

@Juraj-Masiar
Copy link

Just look at the DefinePlugin for Webpack. It will replace all global variables with the specified name. For example this is what I use in the plugins array:

      new webpack.DefinePlugin({
        $IS_FIREFOX: IS_FIREFOX || IS_THUNDERBIRD,
        $NOT_FIREFOX: !IS_FIREFOX,
        $IS_THUNDERBIRD: IS_THUNDERBIRD,
        $IS_CHROME: IS_CHROME || IS_SAFARI,   // to make it simple, Safari will inherit existing Chrome codebase
        $IS_SAFARI: IS_SAFARI,
        $IS_EDGE: IS_EDGE,
        $IS_DEV: IS_DEV,
        $IS_PROD: IS_PROD,
        $IS_WEB: IS_WEB,
        $IS_EXT: !IS_WEB,
        $IS_NODE: IS_NODE,
        $IS_TEST: false,
      }),

But again, terrible testing, terrible inter-project integration.
It may be better to use some environment variables, even if it's at the cost of dead code not being removed from the final build.
Actually the more I think about it, the more I want to do it :D. Especially now when I have 100 other things to do :)

@csandman
Copy link

csandman commented Oct 7, 2021

Do environment variables get loaded in a live browser extension?

@Juraj-Masiar
Copy link

I have no idea :). I use them only in the backend, but that's integrated in the Nestjs framework I use.
But I would say there must be something for Webpack because people want to environment variables during the build time and they for sure want to drop dead code. I remember I was not able to find it though, but it was years ago.

@fregante
Copy link
Contributor

fregante commented Oct 7, 2021

Do you have a consistent way of checking whether or not it is running in Safari in the code

This is a generic StackOverflow question, let's not notify everyone for unrelated requests 😅

@fregante
Copy link
Contributor

fregante commented Oct 7, 2021

Returning to topic, I've been using webextension-polyfill on Chrome, Firefox and Safari (same build) for a year, so no custom implementation is needed. If you want to build it for Firefox/Safari specifically, just don't include it.

This issue probably is just about official support and testing in the repo, but you don't necessarily have to wait for it, as a user.

@csandman
Copy link

csandman commented Oct 8, 2021

This is a generic StackOverflow question, let's not notify everyone for unrelated requests 😅

Sorry about that 😓 the user agent checking works find for the popup but I still haven't been able to find a consistent way to determine which browser a background script is running in (besides the custom webpack scripts @Juraj-Masiar mentioned). That's the only reason I asked here.

@xeenon
Copy link
Contributor

xeenon commented Mar 25, 2024

FYI, Safari does support Promise replies in onMessage listeners (for a couple years now) — since Safari 15.4 (Released 2022-03-14).

I think this issue can be closed.

@Rob--W
Copy link
Member

Rob--W commented Mar 25, 2024

FYI, Safari does support Promise replies in onMessage listeners (for a couple years now) — since Safari 15.4 (Released 2022-03-14).

I think this issue can be closed.

Thanks for checking. It is also listed in the BCD: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/Runtime/onMessage#browser_compatibility

(to those interested, Chrome also expressed interest in implementing Promise support in onMessage)

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