containers-everywhere / contain-google

[Looking for maintainer] - Google Container isolates your Google activity from the rest of your web activity in order to prevent Google from tracking you outside of the Google website via third party cookies.
https://addons.mozilla.org/en-US/firefox/addon/google-container/
Mozilla Public License 2.0
408 stars 50 forks source link

Add option to whitelist domain from containment #108

Closed sents closed 3 years ago

sents commented 3 years ago

This pull request adds an option to include a list of whitelisted domains. It is my first real pull request so I hope I am doing this right.

Note that this would fix #104.

Perflyst commented 3 years ago

Thanks! I will test your patch as soon as possible. Also it needs a quick rebase

@hackerncoder what do you think?

sents commented 3 years ago

I rebased it now.

I've noticed that pressing enter in the extension preferences triggers saving the preferences. As the domains are separated by newlines there is no way to enter multiple domains right now. I would propose to use "," or ";" as separator. Both characters are not allowed as part of domain names.

sents commented 3 years ago

The whitelist option uses a textarea element now, so it is possible to enter multiple domains.

hackerncoder commented 3 years ago

Thanks! I will test your patch as soon as possible. Also it needs a quick rebase

@hackerncoder what do you think?

This is some serious code, and somewhat out of my ballpark (especially since I don't have that much energy right now), but seems fine.

hackerncoder commented 3 years ago

I tried it a little (google.com and blogger.com) and it seems fine.

Perflyst commented 3 years ago

@sents after hackerncoder's test I merged and released this feature. the containment is broken if the whitelisted google urls are empty -> by default the containment is broken

Perflyst commented 3 years ago

Additionally your popup is broken if the preferences are saved with empty whitelist unknown

hackerncoder commented 3 years ago

the containment is broken if the whitelisted google urls are empty -> by default the containment is broken

I can't reproduce this.

sents commented 3 years ago

I'll look into it today.

sents commented 3 years ago

Unfortunately I am not able to reproduce the bugs relating to the empty whitelist. I've tried my standard profile and a fresh profile without any other extensions.

Perflyst commented 3 years ago

In case you downloaded from AMO - I disabled version 1.5.2 there. If you download https://github.com/containers-everywhere/contain-google/releases/download/1.5.2/google_container-1.5.2.zip, create a fresh profile, use debug addons -> load temporary addon you should be able to reproduce it.

sents commented 3 years ago

I installed it on a different computer with Firefox 82 and could reproduce the bug. For some reason whitespace gets inserted into the setting when loading the settings for the first time. Thank you for your tip! I will now look into this.

sents commented 3 years ago

After the fix of #109 leaving the whitelist empty doesn't cause any problems anymore. The problem was caused by the variables relating to whitelist being undefined, when loading the extension for the first time. The checkboxes are also undefined, but this doesn't cause any problems because undefined works with boolean operators.