Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Conversation

@Bisaloo
Copy link
Collaborator

@Bisaloo Bisaloo commented Jul 27, 2017

No description provided.

@Bisaloo
Copy link
Collaborator Author

Bisaloo commented Jul 27, 2017

I think we should review the way tests are currently working to check duplicates.

At the moment, tests will pass even if a target is duplicated in more than one ruleset as long as it is not in more than one ruleset without platform. For example, the domain in this PR was not in duplicate-whitelist.txt.

But this can cause problems. In this case, it was pretty obvious that there is mixed content so the ruleset without platform shouldn't have made it to the repo.

But we can think of more tricky cases, where the mixedcontent is not on the landing page but on deeplinks. In that case, User 1 creates a ruleset without platform="mixedcontent". User 2 sees that the website can be secured but doesn't realize there is already a rule for it (platform="mixedcontent" rules are disabled by default for most users). But User 2 misses the fact that this website has mixed content and creates a ruleset without platform. We are now breaking a website when we could have easily avoided it.

I propose that all duplicates require to be in duplicate-whitelist.txt, even if one of the rulesets they are in has a platform. I know that in some cases, some contributors will secure some resources in a ruleset with platform and the rest in platform="mixedcontent" but adding such domains in the whitelist is trivial and that's what the whitelist is for anyways (not to mention the fact that to my knowledge, no one is doing that anymore. MB used to do it but they were the only one).

CC @J0WI, @jeremyn, @Hainish

@J0WI
Copy link
Contributor

J0WI commented Jul 27, 2017

Thanks!
Could you copy this into a new issue? Do you have any idea how many rules would be affected by this change?

@J0WI J0WI merged commit 5c50873 into EFForg:master Jul 27, 2017
luciancor pushed a commit to luciancor/https-everywhere that referenced this pull request Aug 24, 2017
* Delete Freedombox.xml

* [FreedomBoxFoundation.org] Add mixedcontent platform
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.

2 participants