Skip to content
This repository was archived by the owner on Apr 28, 2020. It is now read-only.

Add support for Firefox to extension#225

Closed
lucacasonato wants to merge 6 commits intocoder:masterfrom
lucacasonato:firefox-extension
Closed

Add support for Firefox to extension#225
lucacasonato wants to merge 6 commits intocoder:masterfrom
lucacasonato:firefox-extension

Conversation

@lucacasonato
Copy link
Copy Markdown
Contributor

@lucacasonato lucacasonato commented Jun 20, 2019

Adding support for Firefox to the extension is relatively simple because both Chrome and Firefox use the WebExtensions API. In our case this means that the same code will work as long as you place the native host manifests into the correct directory for Firefox. I added this to the extension setup command.

Annoyingly Firefox handles its Content Security Policy differently to Chrome for extensions (https://bugzilla.mozilla.org/show_bug.cgi?id=1267027). In a nutshell this means that we can not call ws://127.0.0.1:XXXXX/api/v1/run from the content script itself. This means that the ws needs to be opened in the background script, which can then proxy messages to the content script. I have implemented this too.

I also changed the command to setup the native host manifests to sail setup-extension (because it does both Chrome and Firefox).

@lucacasonato
Copy link
Copy Markdown
Contributor Author

There is no .prettierrc file or similar included, so I formatted it in a way that seemed to fit most. If there is a .prettierrc file anywhere that belongs to this it would be nice if this was added to the repo.

@lucacasonato lucacasonato marked this pull request as ready for review June 20, 2019 14:15
@lucacasonato lucacasonato changed the title Add support for Firefox extension Add support for Firefox to extension Jun 20, 2019
@ammario ammario requested a review from kylecarbs June 20, 2019 19:03
@deansheather
Copy link
Copy Markdown
Member

I think the old command install-for-chrome-ext should be aliased to the new one, even if it gets a deprecated warrning.

@kylecarbs Could you please review this?

@lucacasonato
Copy link
Copy Markdown
Contributor Author

@kylecarbs

Comment thread extension/src/content.ts
@lucacasonato lucacasonato mentioned this pull request Aug 9, 2019
5 tasks
Copy link
Copy Markdown
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the extension stuff is rolled back and an alias is added for the old command name this should be good to merge.

Thanks for the work you did in regards to moving the websocket to the background script, I didn't realize that this had already been done when I did it in the approved hosts PR.

@deansheather
Copy link
Copy Markdown
Member

Working on this in a new PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants