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

Broken? #10

Closed
Happy-Ferret opened this issue Nov 22, 2016 · 6 comments
Closed

Broken? #10

Happy-Ferret opened this issue Nov 22, 2016 · 6 comments

Comments

@Happy-Ferret
Copy link

The current master doesn't seem to work.
At least not for me.

When I add the unminimized distribution to manifest.json (see below), the output is as follows:

Error: Error: Invocation of form runtime.sendNativeMessage(string, string, function) doesn't match definition runtime.sendNativeMessage(string application, object message, optional function responseCallback)

utilizing the minimized version, I receive the following error output

browser-polyfill.min.js:18 Uncaught SyntaxError: Unexpected token {

An excerpt from my manifest.json

"background": {
    "scripts": ["webextension-polyfill/dist/browser-polyfill.js",
                "background.js"]
  },

Browser: Chrome 54.0.2840.99

@rpl
Copy link
Member

rpl commented Nov 22, 2016

@Happy-Ferret I took a look into this and it looks like there are two separate issues:

The first one (the "Invocation Error") seems to be related to a small difference in the definitions of this API method in the Firefox and Chrome JSON schema files:

Given the above schema API method definitions, an extension that wants to be compatible with both Chrome and Firefox have to use a message of type object (which is supported by both the definitions), e.g.:

browser.runtime.sendNativeMessage("ping_pong", {data: "a string message in an object"})
  .then(reply => console.log("Native message reply", reply))
  .catch(err => console.error("Native message error", err));

Let me know it the above workaround fixes the "Invocation Error" for you too.

I'm still looking into the minified file issue (the "SyntaxError"), it looks like like something that can be related to some ES6 syntax not correctly transformed by the additional "minification" step.

@rpl
Copy link
Member

rpl commented Nov 22, 2016

@Happy-Ferret I looked deeper into the "minified file's SyntaxError" part of this issue and it looks like that my initial guess was right: google-closure-compiler seems to be the one that introduces the issue when the sources are minified.

I did an brief experiment by using babili as an alternative minifier that is officially "ES6-aware" and I created a pull request for it in #11.

I tried to load the new minified js built using #11 in a test addon running on Chromium 53.0.2785.143 and it has been loaded successfully.

Can you give a try to #11 and confirm that it fixes this issue for you too?

(and thanks a lot for your detailed bug report, it has been very helpful to track down these issues)

@Happy-Ferret
Copy link
Author

Unfortunately, that didn't solve it. At least not for my use-case.

The following code still produces an error of the form

Invocation of form runtime.sendNativeMessage(string, string, function) doesn't match definition runtime.sendNativeMessage(string application, object message, optional function responseCallback)

suggesting it doesn't quite wrap the promise interface of browser into the chrome callback standard.

background.js

function onResponse(response) {
  console.log("Received: " + response);
}

function onError(error) {
  console.log(`Error: ${error}`);
}

/*
On a click on the browser action, send the app a message.
*/
browser.browserAction.onClicked.addListener(() => {
  console.log("Sending:  ping");
  var sending = browser.runtime.sendNativeMessage(
    "ping_pong",
    "ping");
  sending.then(onResponse, onError);
});

The invocation succeeds flawlessly in Firefox.

@rpl
Copy link
Member

rpl commented Nov 24, 2016

@Happy-Ferret it should work correctly on both Chrome and Firefox if the message parameter (the second one) is an object instead of a string (due to the differences in the schema definitions highlighted in the above comment).

In other words, this will not work correctly on Chrome (but works on Firefox):

var sending = browser.runtime.sendNativeMessage("ping_pong", "ping");

while the following should work correctly on Chrome (and also on Firefox):

var sending = browser.runtime.sendNativeMessage("ping_pong", {myMessageProperty:"ping"});

@Happy-Ferret
Copy link
Author

Thanks!

That did indeed solve it.

I still have the problem that it can't find the host on Google Chrome and if I add a Chrome extension ID it will suddenly work on neither Chrome nor Firefox. But that's clearly beyond the scope of this bug report.

@rpl
Copy link
Member

rpl commented Nov 25, 2016

Thanks!
That did indeed solve it.

@Happy-Ferret I'm glad to hear that! Thanks for confirming that it fixed your issue.

I still have the problem that it can't find the host on Google Chrome and if I add a Chrome extension ID it will suddenly work on neither Chrome nor Firefox. But that's clearly beyond the scope of this bug report.

That's true, it sounds like a compatibility issue in the WebExtensions APIs implementation, which is tracked using bugzilla.

Nevertheless, it sounds like an issue that worth more digging, would you mind to report this issue on bugzilla using url that follows?
(even better if you can attach a minimal set of assets to reproduce the issue, e.g. the native app manifest and a minimal extension to run on both Firefox and Chrome)

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

2 participants