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

Show only rulesets which performed an upgrade #17047

Closed
wants to merge 1 commit into from
Closed

Show only rulesets which performed an upgrade #17047

wants to merge 1 commit into from

Conversation

cschanaj
Copy link
Collaborator

@cschanaj cschanaj commented Nov 8, 2018

As a side effect of #17010, the popup no longer shows rulesets which do not perform any upgrade on HTTPS pages, this PR makes it the default behavior on HTTP pages as well. It also introduce room for performance improvement like #16951.

There was concerns that users might need granular control of all the ruleset states to avoid site breakages, but #16546 is merged so I guess this is less likely an issue.

A controversial side effect could be the fact that default_off rulesets will not be visible anymore. That's why I have closed #16951 before #17010 emerged.

cc @Hainish @jsha @zoracon @Bisaloo @J0WI @jeremyn for opinions

@Bisaloo
Copy link
Collaborator

Bisaloo commented Nov 8, 2018

Related issues: #9688, #12076, #13412

@Bisaloo
Copy link
Collaborator

Bisaloo commented Nov 8, 2018

A controversial side effect could be the fact that default_off rulesets will not be visible anymore.

Currently, most default_off rulesets are terribly outdated. The lack of clear explanations of why a ruleset is default_off (#9906) also make it nearly impossible for users to know if a ruleset can safely be enabled or not (even for technical savvy users).

As far as I know, ruleset maintainers do not merge default_off rulesets anymore, causing their number to steadily decline with time so I don't think that's a huge issue. default_off rulesets could have been useful and it was interesting to keep them around in the UI as a test but it's not practical enough to make sense today IMHO.

@J0WI
Copy link
Contributor

J0WI commented Nov 25, 2018

I'd prefer to keep the default_off rules in the UI, there is no other practical way to enable them.

@Hainish
Copy link
Member

Hainish commented Nov 26, 2018

If the number of default_off rules is declining, I don't see much of a problem in keeping them in the UI as @J0WI suggests.

Relatedly, I've had questions come up over what a Stable Rule is. We might want to change this title to something else, or remove it entirely and only label the Unstable Rules when they are present.

@Hainish
Copy link
Member

Hainish commented Nov 26, 2018

@cschanaj:

As a side effect of #17010, the popup no longer shows rulesets which do not perform any upgrade on HTTPS pages

It's actually worse than that, it doesn't allow you to disable rulesets even if they have performed an upgrade. Try http://freerangekitten.com (our own HTTPS Everywhere test site) for instance.

@Hainish
Copy link
Member

Hainish commented Nov 26, 2018

I've just pushed a stopgap fix for #17087. I'm going to close this PR as well, since the behavior is now consistent.

@Hainish Hainish closed this Nov 26, 2018
@jsha
Copy link
Member

jsha commented Nov 27, 2018

Relatedly, I've had questions come up over what a Stable Rule is. We might want to change this title to something else, or remove it entirely and only label the Unstable Rules when they are present.

Let's just remove the "Stable Rule" text entirely.

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.

5 participants