Only show a ruleset if at least one redirection took place.

Type: code issue
ping @Hainish @cowlicks.

Possibly also if the redirect could have taken place if the request were made over HTTP.

[jeremyn]

As I understand it, a ruleset appears in the UI if the page URL or the URLs of any secondary request match that ruleset. So this issue is already solved if we allow the condition "Possibly also if the redirect could have taken place if the request were made over HTTP." Is that right?

If we don't allow that condition, so that for example Freerangekitten.com would not appear in the UI if you directly visit https://freerangekitten.com , then you wouldn't be able to disable that ruleset, since it's not in the UI to toggle on or off.

@jeremyn No, it simply checks for targets, not for actual possible redirection.

[Bisaloo]

@jeremyn No, it simply checks for targets, not for actual possible redirection.

That's what I observed as well. It was pretty obvious with the google rulesets. Not so much now that some of them have been removed.

Note that this differs from the behaviour of the legacy HTTPS-Everywhere.

[cschanaj]

I think that I can confirm @koops76 and @Bisaloo 's observations. Given a tabId, for each request, we look for rulesets which have matching target. Among these ruleset, if it is active (not default_off), we show it as an entry of "Stable rules"; otherwise, it is regarded as "Experimental rules". So, whether or not these rulesets make a redirection is not taken into account.

While this issue shouldn't be too difficult to fix, I am not sure what is the expected behavior for those default_off rulesets? By definition, they do not make redirections until an user activate them. However, if they do not show up, it might not be possible for non-technical user to activate these rulesets.

cc @Hainish @cowlicks

https-everywhere/chromium/background.js

Lines 285 to 290 in e958e4b

	for (let ruleset of potentiallyApplicable) {
	activeRulesets.addRulesetToTab(details.tabId, ruleset);
	if (ruleset.active && !newuristr) {
	newuristr = ruleset.apply(canonical_url);
	}
	}

Possible fix (not tested)

for (let ruleset of potentiallyApplicable) {
  newuristr = ruleset.apply(canonical_url); // apply() for each ruleset is very inefficient
  if(newuristr) {
    activeRulesets.addRulesetToTab(details.tabId, ruleset);
  }
}

[jeremyn]

All: thanks for the clarification. So is the point of this issue to take into account complications like exclusions and complex rules when deciding when to show a ruleset in the UI?

[Hainish]

Thanks for the issue, and @cschanaj's explanation is correct: we show all potentially applicable rulesets, whether or not they are actually triggered on a page. This is behavior that is consistent with the legacy extension, and I believe should not be altered.

One reason for this is that for a particular page which no redirection has occurred, you may still want to disable (or enable) a potentially applicable ruleset.

#12325 (comment) is related, but instead of hiding the rulesets away in the menu, the suggestion is to limit the number you see in the badge to how many actual redirects happen due to HTTPS Everywhere. This seems most sensible to me.

@Hainish Do you want me to open a PR?

[Hainish]

@koops76 yes, I think that would be useful, thank you.

@Hainish Does Chrome have a "tab load started" event?

[Hainish]

@koops76 https://developer.chrome.com/extensions/webRequest

[pipboy96]

Duplicate of #17047.