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

Migration to service worker initiated #63

Merged
merged 8 commits into from
Apr 30, 2024

Conversation

kabir-afk
Copy link
Collaborator

@kabir-afk kabir-afk commented Mar 19, 2024

This PR arose because of issue#55 (earlier closed with unmerged commits in PR60 due to its inability to work in FF, altho were later fixed by @hugolpz , but have introduced some other changes as well). Apparently upon unpacking the extension file it showed

Failed to load extension
File
~\Downloads\SignIt\SignIt-master
Error
'content_security_policy.extension_pages': Insecure CSP value "https://commons.wikimedia.org" in directive 'object-src'.
Could not load manifest.

The issue could have been resolved by simply removing object-src 'self' https://commons.wikimedia.org; from extension pages but I not only removed it but also updated it as per the syntax of manifest V3 which changed extension_pages to a dictionary and introduced the sandbox key. Now upon unpacking it won't show CSP related issues in chrome dev mode . In FF it will show warning about sandbox but nothing more than that. .Referenced Click here to see changes
Also with the removal of insecure CSP we wont be needing the CSP header bypass logic as well, not for chrome at the very least inside sw.js .

Additional Info on Cross browser background scripts

Regarding background scripts , apparently chrome and FF have different entrypoints , FF uses background pages by using "background":{"scritps":[background.js]}, but chrome no longer accepts that and has a different entry point , something like "background":{"service_worker" : "background.js"}. So in order to make cross browser background-scripts we can add both entrypoints to make it work. We can further add guards to our manifest for users with older versions of chrome/FF to make them get these updates( mentioned here skip to 23:33 to see this snippet).

"browser_specific_settings": {
"gecko": {
"id": "script-on-click@example",
"strict_min_version": "121.0a1"
},
"gecko android": {
"strict min version": "121.0a1"
},
"minimum_chrome_version": "121"
},

Although I doubt that'll unlikely be the case , not to mention it shows warning in chrome as deeming "browser_specific_settings" as unidentified.

No need for polyfills : Click here to see changes

In the past (manifest V2), promises worked in Firefox but not in Chrome. So, we had to use polyfills. Now (V3), Chrome APIs support promises, removing the need for polyfills.

Typechecking browser in popup and introduction of sw.js(service-worker) : Click here to see changes

When working with service_worker, we need to pass messages in order to communicate with our content scripts . . . which could be solely achieved by using chrome apis, due to which I changed the way we used var browser earlier, so that we can interchange with chrome and FF runtime.

Making sw.js seperately because , when I tried runnig the same script in background.js , service worker failed to register and exited with code 15,might change later my understanding is better, but for now we can see an active service worker . . . Earlier it also used to show

error : background page not found

That is not so the case any longer . . . a background page is certainly recognised.

Changes dated 31st March

Well they are descriptive enough by their commit message headings , also I have provided in depth explanation of the changes I have made inside the codebase through comments, there I have solved issues related to seeing modal, importing banana module inside sw,js as well as exporting it to popup.js while maintaining its functionality like earlier, but would still like to mention some points where code is still broken and responds weirdly.

Issue 1 : Modal doesn't appear right away when accessed through contextMenu or hintIcon

Reproduce : click on Lingua Libre SignIt in contextMenu , then you might see hint icon , click on it and then your modal might appear. . . .still needs to be worked upon but something's better than nothing.

As far as the state and other variables are concerned , they need to persisted in chrome.storage.local but for now I've sent them through an object whenever getBackground message is passed , similar has to be done for all the functions that are being accessed in content scripts, will work on that later when I have figured it out .

Issue 2 : The fetch API post requests won't work , I get status code 405 : Method not allowed, despite being an ideal approach to replace jQuery which is incompatible with service_worker.

Approach : We can leverage the power of offscreen API and use the jQuery $.post requests there , and can make it communicate with sw.js by passing messages like we normally do with other content scripts, haven't implemented yet but should work just fine.

Whole popup wont be visible but something like this should be there

Screenshot (39)

Again apologies for the long PR , should've raised them on a daily basis

@kabir-afk kabir-afk requested a review from hugolpz March 19, 2024 12:48
@kabir-afk kabir-afk changed the title Migrating to service worker initiated Migration to service worker initiated Mar 20, 2024
@kabir-afk kabir-afk mentioned this pull request Apr 30, 2024
6 tasks
@hugolpz hugolpz merged commit 550e7b0 into lingua-libre:master Apr 30, 2024
@hugolpz
Copy link
Member

hugolpz commented Apr 30, 2024

@kabir-afk : long due merged !
My mind is not in this code at the moment, but it's good to see you going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants