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

runtime.sendMessage/onMessage #58

Closed
hgwood opened this issue Sep 26, 2017 · 16 comments · Fixed by #64
Closed

runtime.sendMessage/onMessage #58

hgwood opened this issue Sep 26, 2017 · 16 comments · Fixed by #64

Comments

@hgwood
Copy link

hgwood commented Sep 26, 2017

Hello

I'm running into problems when trying to use browser.runtime.sendMessage and browser.runtime.onMessage with this polyfill on Chrome 61.

I have this code that works fine:

// in background script, run when the extension is loaded
chrome.runtime.onMessage.addListener((message, sender) => {
  console.log("background script got message", message, "from", sender);
});

// in content script, run when the user presses a button
chrome.runtime.sendMessage({type: "event", eventName: "THE_EVENT"});

If I replace chrome with browser in the background script, I get this in its console:

image

and the listener is not run.

Independently, if I replace chrome with browser on the content script side, I get this in the console of the page:

image

but the listener is run.

If I replace chrome with browser on both sides, then the content script says this instead:

image

and the background script message remains unchanged.

I'm new to writing browser extensions so I'm not sure if this might be a bug in the polyfill or if not using the API quite right.

@hgwood
Copy link
Author

hgwood commented Sep 26, 2017

I think the two errors always happen but they are not reported when using chrome because they are stored in runtime.lastError, while the polyfill uses rejected promises, which do appear as console errors. So only the background script side remains a problem.

@Rob--W
Copy link
Member

Rob--W commented Sep 28, 2017

(tested with Chrome 61 and latest polyfill version (0.1.2))

If I use chrome at the content script and browser at the background script, the message gets through without errors.
If I use browser at the content script and chrome or browser at the background script, the message also gets through, but the error message is always set:

The message port closed before a response was received.

I think that we should catch this specific error and ignore it (treat it as no response).

@RuiNtD
Copy link

RuiNtD commented Oct 1, 2017

I got this error from the content script:

(CONTENT_SCRIPT context for ljhjkdmpfemkkhfhfakknbjogcopkebg) extensions::lastError:98: Uncaught Error: runtime namespace is null or undefined{Error: runtime namespace is null or undefined
    at assertRuntimeIsAvailable (extensions::lastError:98:11)
    at Object.clear (extensions::lastError:84:3)
    at clearLastError (extensions::messaging:62:19)
    at dispatchOnDisconnect (extensions::messaging:379:9)}

I only got the extension working with chrome in the background script and browser in the content script, but the content script error happens whenever I have browser in the content script. The trace stack is different depending on if I use chrome or browser in the background script, but the actual error remains the same. However, I never got the "Uncaught (in promise)" errors. My tests were done with Chrome 61 and polyfill version 0.2.0.

@jowa0720
Copy link

jowa0720 commented Oct 2, 2017

For me, it does not matter which combination of browser or chrome I use. In all cases the onMessage listener set in the background script is not executed.

If I use browser in the background script, chrome console ouputs the following when loading the background page:

error1

If I use chrome in the the background script, the console adds an additional error:

error2

I am using Chrome 61 and polyfill version 0.2.0.

@Rob--W
Copy link
Member

Rob--W commented Oct 2, 2017

@rpl Could you investigate? I think that it might be related to #57.

@ahmadassaf
Copy link

ahmadassaf commented Oct 3, 2017

i can confirm that #57 fixes the issues mentioned above in a sense that browser both in content and background script will be able to communicate. However, the issue now is the sendResponse in case needed to send back an answer does not work which i believe is somehow related to #16 and a fix has been addresses in #22

@hgwood
Copy link
Author

hgwood commented Oct 3, 2017

@ahmadassaf @Rob--W I was already using what is now named 0.2.0 when I witnessed and opened the issue, as I built the polyfill from master, so #57 was already included. I do not use sendResponse. To confirm this, I have updated by local master and rebuilt the polyfill, then diffed the new version against mine. The only difference is the version number and date in the header comment.

@hgwood
Copy link
Author

hgwood commented Oct 3, 2017

It appears Chrome 61 might have been updated since I opened the issue. Without changing my code, here is what I now get in the content script console when I use browser on the content script side:

image

Looks similar to @FayneAldan and @jowa0720, but slightly different. Might be because of slightly different versions of Chrome. My version right now is 61.0.3163.100 on Windows 10 x64.

Anyway, this error does not stop the message from going through. However, the message does not get through when I use browser on the background script side. In all other cases it does. Polyfill version is still 0.2.0.

@ahmadassaf
Copy link

@hgwood i have just noticed something in my background.js that might be useful. Basically before coming across #57 i had the following:

const messageListner = typeof InstallTrigger !== 'undefined' ?  browser.runtime : chrome.extension;

Since i want my extension to work mainly in Chrome and FF, i added this check for FF and then assigning the message listener accordingly. I do no use messageListner anywhere in my codebase now .. but removing it make me get the error again .. just though to share this if it helps

@RuiNtD
Copy link

RuiNtD commented Oct 4, 2017

So, that would mean there's something wrong in the Proxy's get method, right? Why do we even need proxies? Why not just make an object with the available methods?

@midzelis
Copy link

midzelis commented Oct 6, 2017

Just want to chime in and say that I'm currently hitting this issue. I'm going to see if browser specific workaround posted by ahmadassaf will work for me.

@DamienCassou
Copy link

My code

  • works perfectly with webextension-polyfill 0.1.2
  • fails with "Previous API instantiation failed." and "Could not establish connection. Receiving end does not exist." with version 0.2.0 and Chromium 60

The popup does:

  await browser.tabs.executeScript(tabId, {file: '/browser-polyfill.min.js'})
  await browser.tabs.executeScript(tabId, {file: '/content-script.js'})
  await browser.tabs.sendMessage(tab.id, {action: 'getHeadings'})

and content-script.js is:

function messageReceived (message) {
  return Promise.resolve(...)
}

browser.runtime.onMessage.addListener(messageReceived)

I suggest using 0.1.2 for now.

@hgwood
Copy link
Author

hgwood commented Oct 9, 2017

I confirm that 0.1.2 also works for me.

@midzelis
Copy link

midzelis commented Oct 9, 2017

My workaround was to create my own polyfill, using https://github.com/tfoxy/chrome-promise as a start, and filling in the non-callback ones with a simple re-bind from browser to chrome.

@rpl rpl closed this as completed in #64 Oct 11, 2017
@rpl
Copy link
Member

rpl commented Oct 12, 2017

The fix for this issues (#64) has been included in the 0.2.1 release (https://github.com/mozilla/webextension-polyfill/releases/tag/0.2.1) and published on npm.

Rob--W added a commit to Rob--W/webextension-polyfill that referenced this issue Oct 23, 2017
Originally, the polyfill created a Proxy with the original API object as
the target. This was changed to `Object.create(chrome)` because not
doing so would prevent the `browser.devtools` API from working because
the devtools API object is assigned as a read-only & non-configurable
property (mozilla#57).

However, that action itself caused a new bug: Whenever an API object
is dereferenced via the `browser` namespace, the original API is no
longer available in the `chrome` namespace, and trying to access the
API through `chrome` returns `undefined` plus the "Previous API
instantiation failed" warning (mozilla#58).
This is because Chrome lazily initializes fields in the `chrome`
API, but on the object from which the property is accessed, while
the polyfill accessed the property through an object with the
prototype set to `chrome` instead of directly via chrome.

To fix that, `Object.create(chrome)` was replaced with
`Object.assign({}, chrome)`. This fixes both of the previous issues
because
1) It is still a new object.
2) All lazily initialized fields are explicitly initialized.

This fix created a new issue: In Chrome some APIs cannot be used even
though they are visible in the API (e.g. `chrome.clipboard`), so
calling `Object.assign({}, chrome)` causes an error to be printed to
the console (mozilla#70).

To solve this, I use `Object.create(chrome)` again as a proxy target,
but dereference the API via the original target (`chrome`) to not
regress on mozilla#58.
Besides fixing the bug, this also reduces the performance impact
of the API because all API fields are lazily initialized again,
instead of upon start-up.

This fixes mozilla#70.
Rob--W added a commit to Rob--W/webextension-polyfill that referenced this issue Feb 26, 2018
Originally, the polyfill created a Proxy with the original API object as
the target. This was changed to `Object.create(chrome)` because not
doing so would prevent the `browser.devtools` API from working because
the devtools API object is assigned as a read-only & non-configurable
property (mozilla#57).

However, that action itself caused a new bug: Whenever an API object
is dereferenced via the `browser` namespace, the original API is no
longer available in the `chrome` namespace, and trying to access the
API through `chrome` returns `undefined` plus the "Previous API
instantiation failed" warning (mozilla#58).
This is because Chrome lazily initializes fields in the `chrome`
API, but on the object from which the property is accessed, while
the polyfill accessed the property through an object with the
prototype set to `chrome` instead of directly via chrome.

To fix that, `Object.create(chrome)` was replaced with
`Object.assign({}, chrome)`. This fixes both of the previous issues
because
1) It is still a new object.
2) All lazily initialized fields are explicitly initialized.

This fix created a new issue: In Chrome some APIs cannot be used even
though they are visible in the API (e.g. `chrome.clipboard`), so
calling `Object.assign({}, chrome)` causes an error to be printed to
the console (mozilla#70).

To solve this, I use `Object.create(chrome)` again as a proxy target,
but dereference the API via the original target (`chrome`) to not
regress on mozilla#58.
Besides fixing the bug, this also reduces the performance impact
of the API because all API fields are lazily initialized again,
instead of upon start-up.

This fixes mozilla#70.
rpl pushed a commit that referenced this issue Mar 12, 2018
Originally, the polyfill created a Proxy with the original API object as
the target. This was changed to `Object.create(chrome)` because not
doing so would prevent the `browser.devtools` API from working because
the devtools API object is assigned as a read-only & non-configurable
property (#57).

However, that action itself caused a new bug: Whenever an API object
is dereferenced via the `browser` namespace, the original API is no
longer available in the `chrome` namespace, and trying to access the
API through `chrome` returns `undefined` plus the "Previous API
instantiation failed" warning (#58).
This is because Chrome lazily initializes fields in the `chrome`
API, but on the object from which the property is accessed, while
the polyfill accessed the property through an object with the
prototype set to `chrome` instead of directly via chrome.

To fix that, `Object.create(chrome)` was replaced with
`Object.assign({}, chrome)`. This fixes both of the previous issues
because
1) It is still a new object.
2) All lazily initialized fields are explicitly initialized.

This fix created a new issue: In Chrome some APIs cannot be used even
though they are visible in the API (e.g. `chrome.clipboard`), so
calling `Object.assign({}, chrome)` causes an error to be printed to
the console (#70).

To solve this, I use `Object.create(chrome)` again as a proxy target,
but dereference the API via the original target (`chrome`) to not
regress on #58.
Besides fixing the bug, this also reduces the performance impact
of the API because all API fields are lazily initialized again,
instead of upon start-up.

This fixes #70.
@ElForastero
Copy link

I'm not sure what went wrong, but I get this error too.
v0.4.0.

Both background and content scripts use browser.runtime to send and receive messages.

image

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