support Chrome via WebExtension browser API Polyfill#10
support Chrome via WebExtension browser API Polyfill#10gianpaj wants to merge 3 commits intoa13o:masterfrom
Conversation
a13o
left a comment
There was a problem hiding this comment.
So far LGTM. Promisified web-ext API is a benefit itself and if it leads to cross-browser support even better.
| contentScript.files | ||
| .filter(file => file.match(/.*\.css$/)) | ||
| .map(file => `${contentScript.folder}/${file}`) | ||
| .map(file => browser.tabs.insertCSS(null, { file })); | ||
|
|
||
| GLOBAL_SCRIPTS.map(file => browser.tabs.executeScript(null, { file })); | ||
| contentScript.files | ||
| .filter(file => file.match(/.*\.js$/)) | ||
| .map(file => `${contentScript.folder}/${file}`) | ||
| .map(file => browser.tabs.executeScript(null, { file })) |
There was a problem hiding this comment.
This was previously done in browser.contentScripts.register because that method uses the matches array to only inject the css/scripts into tabs where the URL matches the pattern.
I think that previous behavior is worth keeping so that as the project grows it doesn't inject more and more unnecessary css/scripts into each tab.
Is this change needed to work with the browser API package?
There was a problem hiding this comment.
I see. I'm new to these browser.* API methods.
The browser.contentScripts.register doesn't seem to work on Chrome, even after the polyfill. I'll do some more testing on to why it's that
There was a problem hiding this comment.
I looked into this today. browser.contentScripts.register isn't supported by Chrome, so the polyfill doesn't implement it. It only polyfills Chrome's existing api and nothing else.
It's possible that the script loading strategy needs to change to support Chrome. This extension will have to filter the content scripts before injecting them with the cross-platform browser.tabs.executeScript.
I've been slowly working on a larger refactor of the extension that redoes how it loads content scripts, so I can move all the supported sites to optionalPermissions. I think Chrome support should wait for this refactor to finish since it uses browser.tabs.onActivate to determine which scripts to inject, which is the exact functionality needed to move away from leaning on matches in browser.contentScripts.register.
There was a problem hiding this comment.
OK. makes sense. I tried another polyfill and keeping browser.contentScripts.register but didn't work
https://github.com/fregante/content-scripts-register-polyfill
I followed the instructions from the Mozilla repo:
https://github.com/mozilla/webextension-polyfill
Test on Twitter all the CSS (broken/outdated at the moment)