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

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

Closed

Description

Type: code issue
ping @Hainish @cowlicks.

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

Activity

  1. jeremyn commented on Oct 29, 2017

    @jeremyn
    Contributor

    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.

  2. Bisaloo commented on Oct 30, 2017

    @Bisaloo
    Collaborator

    @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.

  3. cschanaj commented on Oct 30, 2017

    @cschanaj
    Collaborator

    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

    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);
      }
    }
  4. jeremyn commented on Oct 30, 2017

    @jeremyn
    Contributor

    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?

  5. Hainish commented on Oct 31, 2017

    @Hainish
    Member

    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.

  6. Hainish commented on Oct 31, 2017

    @Hainish
    Member

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

  7. Hainish commented on Oct 31, 2017

    @Hainish
    Member
  8. pipboy96 commented on Sep 17, 2019

    @pipboy96
    Contributor

    Duplicate of #17047.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions