Chrome extension to use Manifest V3#37
Conversation
| if (typeof asa !== 'undefined') { | ||
| self.apiSecret(asa); | ||
| } | ||
| var apiKeyViewModel = { |
There was a problem hiding this comment.
you'll see changes to all of the Knockout viewmodels due to this: https://developer.chrome.com/docs/extensions/develop/migrate/improve-security#remove-execution-of-strings
Manifest v3 has a new rule for Content Security Policy disables eval and new Function, and Knockout's default binder uses new Function to parse bindings.
So I need to make changes to all the viewmodels to use custom binding: https://github.com/brianmhunt/knockout-secure-binding
There was a problem hiding this comment.
Yeah Knockout is probably a fragile choice for a Chrome extension for a number of reasons, but that makes sense.
| @@ -1,22 +1,4 @@ | |||
| chrome.runtime.onInstalled.addListener(function (details) { | |||
There was a problem hiding this comment.
this is moved to the serviceworker.
some functions are kept here intentionally (ones that still need access to the DOM)
https://developer.chrome.com/docs/extensions/develop/migrate/to-service-workers#changes-over-bg
| self.showHelpLink(false); | ||
| self.newInstall(false); | ||
| self.UrlApiKeys = ('https://my.geni.us/tools#api-section'); | ||
| chrome.storage.local.get(["defaultGroup"]).then((group) => { |
There was a problem hiding this comment.
there's changes to ALL expressions that use localStorage to using chrome.storage.local due to this:
https://developer.chrome.com/docs/extensions/develop/migrate/to-service-workers#convert-localstorage
The web platform's Storage interface (accessible from window.localStorage) cannot be used in a service worker. To address this do one of two things. First, you can replace it with calls to another storage mechanism. The chrome.storage.local namespace will serve most use cases, but other options are available.
| @@ -1,4 +1,4 @@ | |||
| chrome.extension.onMessage.addListener(function (msg, sender, sendResponse) { | |||
| chrome.runtime.onMessage.addListener(function (msg, sender, sendResponse) { | |||
There was a problem hiding this comment.
chrome.extension.onMessage is now unsupported, needs to be replaced:
https://developer.chrome.com/docs/extensions/develop/migrate/api-calls#replace-unsupported-apis
| @@ -0,0 +1,129 @@ | |||
| chrome.runtime.onInstalled.addListener(async function (details) { | |||
There was a problem hiding this comment.
most of the functions here are moved from the background script background.js since we need to be using service worker now in manifest v3
https://developer.chrome.com/docs/extensions/develop/migrate/to-service-workers#differences-with-sws
| "version": "1.0.6", | ||
|
|
||
| "browser_action": { | ||
| "action": { |
There was a problem hiding this comment.
| "content_scripts": [{ | ||
| "matches": ["http://*/*", "https://*/*", "*://*/*"], | ||
| "js": ["js/openDialog.js"] | ||
| "js": ["js/jquery.min.js", "js/servicestack.js", "js/GeniusLinkServiceClient.js", "js/openDialog.js", "js/background.js"] |
There was a problem hiding this comment.
these are the scripts that needs to be accessible by the serviceworker
| "js/frame.js", | ||
| "alertDoneInside.html", "alertLoadingInside.html", "alertDoneOutside.html", "alertLoadingOutside.html", "css/vsprites.svg", "css/main.css", "js/bootstrap.min.js" | ||
| { | ||
| "resources": [ |
There was a problem hiding this comment.
| "background": { | ||
| "scripts": ["js/background.js", "js/jquery.min.js", "js/jquery-ui.min.js", "js/bootstrap.min.js", "js/bootbox.min.js", "js/utilities.js", "js/servicestack.js", "js/GeniusLinkServiceClient.js"], | ||
| "persistent": true | ||
| "service_worker": "js/serviceWorker.js" |
There was a problem hiding this comment.
|
|
||
| "content_security_policy": "script-src 'self' 'unsafe-eval' https://ssl.google-analytics.com; object-src 'self'", | ||
| "content_security_policy": { | ||
| "extension_pages": "script-src 'self' 'wasm-unsafe-eval'; object-src 'self';" |
There was a problem hiding this comment.
https://developer.chrome.com/docs/extensions/develop/migrate/improve-security#update-csp
Manifest V3 disallows certain content security policy values in the "extension_pages" field that were allowed in Manifest V2. Specifically Manifest V3 disallows those that allow remote code execution. The script-src, object-src, and worker-src directives may only have the following values: self, none, wasm-unsafe-eval
| <div style="padding: 20px"> | ||
| <div style="text-align: center;"> | ||
| <button class="btn-gl-blue" style="width: 250px; font-size: 13px;" data-bind="event: { click: createLinkFromButton }">New link from current page</button> | ||
| <button class="btn-gl-blue" style="width: 250px; font-size: 13px;" data-bind="click: createLinkFromButton">New link from current page</button> |
There was a problem hiding this comment.
This is within an extension? Will this scale to the user's preferences (re: font size)?
There was a problem hiding this comment.
yes, this is within the extension. also AFAIK, no. but @ssundheim might know?
| } | ||
| $('#back').on('click', 'a', function () { | ||
| window.location.href = window.history.back(1); | ||
| }); |
There was a problem hiding this comment.
hmm... probably could just a button with that click action?
|
|
||
|
|
||
| var lastLinksModel = new lastLinksViewModel(); | ||
| if (typeof testModel === 'undefined') { |
There was a problem hiding this comment.
equivalent to if (testModel === undefined) -- casting to a string should not be necessary
There was a problem hiding this comment.
this error shows up if we do it that way, so since this is existing code, I'm keeping it as is for now, we can re-investigate and clean up later:
|
|
||
| setTimeout(function () { | ||
| iframe.remove(); | ||
| }, 5000); |
There was a problem hiding this comment.
tbh I don't know what this is, I didn't initially write this app, but since this is existing, I don't want to mess with it. maybe @ssundheim knows?
| @@ -43,5 +39,4 @@ chrome.extension.onMessage.addListener(function (msg, sender, sendResponse) { | |||
| }, 6000); | |||
There was a problem hiding this comment.
Another magic number. Documentation?
There was a problem hiding this comment.
also don't know what this is and why this number. pinging @ssundheim
matthew-dean
left a comment
matthew-dean
left a comment
There was a problem hiding this comment.
Ideally, the .js files wouldn't be dumping variables into the global scope. Are variables that the extension runs available to the host web page?
2 participants
https://geniuslink.atlassian.net/browse/ENG-99?atlOrigin=eyJpIjoiODMzNDVhMzAzODY2NDZmZThhOWU1MzAxMzkwMzVlNTciLCJwIjoiaiJ9