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

Firefox support #2

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Firefox support #2

wants to merge 6 commits into from

Conversation

xileftenurb
Copy link

No description provided.

@tomer8007
Copy link
Owner

Looks good, but - are you running this on Firefox, and the extension successfully blocks ads?

@xileftenurb
Copy link
Author

Well, truth is, I did not test it, I don't know how...

@tomer8007
Copy link
Owner

You can test this using Firefox's developer page. You have instructions for example here.

@xileftenurb
Copy link
Author

oh thank, I will check that

@xileftenurb
Copy link
Author

I tested it (finally using the instruction found here -> https://extensionworkshop.com/documentation/develop/temporary-installation-in-firefox/

and it fails because it does not recognize "wsHook" as a variable.

What puzzle me is that this variable does not seem to exist in firefox, nor in chrome (when tapping "wsHook" in inspector, both return me undefined variable), but chrome does not seem to throw any error while running the script.

@tomer8007
Copy link
Owner

This variable is defined here:


Perhaps there is a problem with the script injection on page load. You can take a look at the other addon here, in which it works. Hopefully this helps.

@xileftenurb
Copy link
Author

xileftenurb commented May 6, 2021

I found the root of the problem ->
Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”). content_script.js:35:43

not sure how this could be fixed, since it will block any script injection.

@tomer8007
Copy link
Owner

Actually this makes me think about how it actually worked in Chrome so far, so I've pushed 66ad20f for completeness. Please try to see if it fixes anything on Firefox now.

@lorenanunez
Copy link

I downloaded the code and compiled for testing into Firefox, and it works. Can I create a pull request for uploading the manifest updated for working also on Firefox?

@tomer8007
Copy link
Owner

@lorenanunez You can open a separate pull request, given that you actually get ads blocked on Firefox

@mackdroid
Copy link

image
im encountering the same issue after installing from the main branch, installing on firefox, changing all instances of chrome -> browser and extension.getURL -> runtime.getURL

@mackdroid
Copy link

a possible workaround would be disabling content security policy browser wide, but i dont think thats a good idea

@mackdroid
Copy link

image
still occurs

@mackdroid
Copy link

oh i forgot to restart the browser, well anyways setting security.csp.enable to false in about:config and the extension works as flawlessly

@tomer8007 tomer8007 changed the title Patch 1 Firefox support Nov 27, 2021
@RodrigoDiazDVivar
Copy link

@tomer8007 would you consider uploading it to mozillas page please?

@tomer8007
Copy link
Owner

@RodrigoDiazDVivar It was already uploaded by another user, but looks outdated: https://addons.mozilla.org/he/firefox/addon/spotiads-by-tomer-public/

@RodrigoDiazDVivar
Copy link

@tomer8007 yes, i already had installed it, but it's outdated and doesn't work, sadly. That's why i came to ask you for it since you are the first one who would know notice about any new upgrades (also it's your addon too) :)

@tomer8007
Copy link
Owner

I don't want to upload it to the Firefox store right now because it will require more maintenance from my side. You are welcomed to try to port the new version yourself, it should be little work.

@RodrigoDiazDVivar
Copy link

ok thank you no problem. i will try it some day :)

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.

5 participants