Skip to content
This repository has been archived by the owner on Jul 26, 2020. It is now read-only.

Events are only listened in the same context that requested them #1

Open
fregante opened this issue Jul 1, 2019 · 20 comments
Open
Labels
enhancement New feature or request

Comments

@fregante
Copy link
Owner

fregante commented Jul 1, 2019

This polyfill will exclusively work if permissions are requested/removed from the same page where the listener is. That means, if you run chrome.permissions.request in the background page, only the same exact page will receive the event.

This works

// In background.js
chrome.contextMenus.onClicked.addListener(() => {
  chrome.permissions.request({
    origins: [`https://youtube.com/*`]
  });
});

// ALSO in background.js
chrome.permissions.onAdded.addListener(({ origins }) => {
  if (!origins || origins.length === 0) {
    return;
  }

  console.log("New origins!", origins);
});

This doesn't

If you want to request from options.html or popup.html, it won't work.

// In options.html
document.querySelector(".add-youtube").addEventListener("click", () => {
  chrome.permissions.request({
    origins: [`https://youtube.com/*`]
  });
});
// In background.js
chrome.permissions.onAdded.addListener(({ origins }) => {
  if (!origins || origins.length === 0) {
    return;
  }

  console.log("New origins!", origins);
});

Add your interest in this issue or send a PR to add support via runtime.sendMessage

@fregante fregante added the enhancement New feature or request label Jul 3, 2019
@namukang
Copy link

Hey, I'm interested in this use case! Does #4 support the any context case or is it still a work in progress? Thanks for creating this polyfill.

@fregante
Copy link
Owner Author

fregante commented Nov 16, 2019

It should support any context that supports chrome.runtime.onMessage but I haven’t tested it at all. Would you be able to give it a try?

@namukang
Copy link

Sure thing! I'll give it a go and let you know how it works out.

@namukang
Copy link

I'm having trouble with setting this up and wondering if you have any ideas.

I'm trying to use the any-context branch that you've set up by running
npm install fregante/webext-permissions-events-polyfill#any-context

When I try to import the package using import 'chrome-permissions-events-polyfill';, I get the following error:

ERROR in ./src/background/background.ts
Module not found: Error: Can't resolve 'chrome-permissions-events-polyfill' in '/Users/dskang/Code/hide-likes-everywhere/src/background'
 @ ./src/background/background.ts 2:0-44

Looking in node_modules, I'm seeing that when installing from a GitHub branch, all the source files are missing:
Screen Shot 2019-11-22 at 11 37 22 AM

Any thoughts on how I can get this set up?

@fregante
Copy link
Owner Author

Ah yes you'd have to build it first, you can install directly. Probably something like:

git clone https://github.com/fregante/webext-permissions-events-polyfill.git
cd webext-permissions-events-polyfill
git fetch origin
git checkout -b any-context origin/any-context
npm i
npm run build
npm link

And then in your project

npm link webext-permissions-events-polyfill

@namukang
Copy link

namukang commented Nov 22, 2019

I got around the above issue by just copying and pasting the entire file to test it out, but I realized I won't be able to use it because I'm using webextension-polyfill which uses browser instead of chrome so I get the error that browser.permissions.onAdded is undefined even with the polyfill.

Since the API for browser is different from chrome (e.g. browser.permissions.request doesn't take a callback), I'm planning to just use message passing using browser.runtime.sendMessage to invoke the permissions listener from a different context.

Even though I don't think I can use this polyfill, thanks for putting it together!

@fregante
Copy link
Owner Author

fregante commented Nov 22, 2019

This polyfills doesn't use the browser API but the chrome API, which is available in both browsers regardless of webextension-polyfill

In Refined GitHub I'm using this polyfill together with webextension-polyfill and I'm calling browser.permissions.onAdded.addListener

https://github.com/sindresorhus/refined-github/blob/2d80172ffaee2e2290a43e42040f37bcc0fad820/source/options-storage.ts#L79

@fregante
Copy link
Owner Author

That extension works both in Firefox and Chrome:

  • in Chrome browser.permissions.onAdded.addListener just mirrors chrome.permissions.onAdded.addListener via webextension-polyfill
  • in Firefox browser.permissions.onAdded.addListener is added by this polyfill explicitly

@fregante
Copy link
Owner Author

If browser.permissions.onAdded is undefined in Firefox, then this code isn't running.

@namukang
Copy link

Hm, gotcha. I'll play around with it a bit more and let you know it goes. Thanks!

@namukang
Copy link

namukang commented Nov 22, 2019

So I've been banging my head against the wall for the last few hours trying to figure this out (thought maybe it had to do with me using webextension-polyfill-ts instead of webextension-polyfill) but it had to do with the fact that the any-context branch is behind master and doesn't include the code to add onAdded to browser.permissions.

Once I rebased any-context onto master, everything worked as intended and I was able to confirm that the polyfill works across contexts. Whew, thought I was taking crazy pills for a while there!

@fregante
Copy link
Owner Author

fregante commented Jan 8, 2020

Indeed, sorry for the misunderstanding, I didn't realize that I had made those important changes after I wrote that PR.

I just published a prerelease under 1.0.2-1. If you're still on this issue could you test it out?

npm install webext-permissions-events-polyfill@1.0.2-1

@namukang
Copy link

namukang commented Jan 8, 2020

Sure thing, I'll give it a try and let you know how it goes.

@namukang
Copy link

namukang commented Jan 8, 2020

Works great! Thanks again for your work on this. 🙏🏽

@fregante
Copy link
Owner Author

fregante commented Jan 9, 2020

Does it work correctly across contexts? I should test it out a bit more

@namukang
Copy link

namukang commented Jan 9, 2020

Yup, that was the part that I tested (but of course, feel free to test more).

I requested the permission from a non-background page and the onAdded listener worked from the background script.

@fregante
Copy link
Owner Author

fregante commented Jan 9, 2020

It kinda works but it's also buggy. Removing a permission results in 3 callbacks:

  1. onAdded with the object {origins: ['https://removed'], permissions:[]} 🤷‍♂️
  2. onRemoved with the object {origins: ['https://removed']}
  3. onAdded with the object {origins: ['https://removed']}

Needs more work

@namukang
Copy link

Ah, definitely should've mentioned that I only tested adding a permission and just checked if it worked as expected end-to-end so I'm glad you tested it much more thoroughly than I did!

@fregante
Copy link
Owner Author

fregante commented Jun 3, 2020

Firefox 77 is out with native support!

@namukang
Copy link

namukang commented Jun 3, 2020

Amazing! Thanks @fregante.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants