Order rulesets by names in popup, Fix #11986

[cschanaj]

EDIT: It seems that there can be more than 1 main_frame rulesets, please do not merge yet. Luckily, it doesn't matter because the earlier main_frame rulesets do not perform rewrites. Otherwise, the sorting/ insertion logic would be over complicated given the advantages this enhancement can bring us.

this PR makes rulesets shown in the popup sorted by names. Close #11986

Test performed (possibly more needed...)

• when extension is disabled, counter not shown
• toggles checkbox to enable/ disable ruleset should works
• counters should display the correct numbers
  • visit https://freerangekitten.com/, only 1 ruleset shown
  • visit https://edition.cnn.com/, more than 1 ruleset shown; CNN.com (partial) shown on the top;
  • visit http://time.com/ (HTTP), more than 1 ruleset shown; TIME.com (partial) NOT shown on the top
  • visit https://www.youtube.com/, more than 1 ruleset shown; YouTube NOT shown on the top (preloaded?)
• rulesets are sorted by name case-insensitively, in both stableRules and unstableRules

Not sure if this should be introduced in the next release as well (given #14600).

cc @Hainish @J0WI @jeremyn @Bisaloo

[jeremyn]

@cschanaj I tried this out in a browser at https://www.cnn.com and it seems to do the job, good work!

I did not exhaustively test it, but here are some things I noticed:

• The sort should probably be case-insensitive, but it isn't. For example at https://www.cnn.com it, in my opinion incorrectly, sorts ads-Twitter.com after Twitter.com.

• The current sort logic appears to list the rulesets in the order they are activated. This has the nice effect of putting the ruleset corresponding to the URL in the address bar at the top of the list. For example, currently CNN.com (partial) is the first ruleset listed at https://www.cnn.com. With the new approach, CNN.com (partial) is buried in the middle of the list. I'm not sure what to propose. We could just move the first ruleset to the top without comment.

• It would be nice if the ruleset list were updated in real-time, meaning if you have the HTTPS Everywhere UI open while the page is loading, you see the rulesets update while you are looking at the UI. It doesn't do that currently either, so this would be an improvement above-and-beyond the changes you're making. This is of course possible for JavaScript on a normal webpage but maybe it's not possible for extensions, or maybe it would require a ton of extra work.

I also have some minor code suggestions, such as: you should prefer === to ==; avoid the Pythonic if (rulesets) {} "false-y" check; you can still use const even if you are modifying a member of the value. I haven't reviewed and understood the code logic well enough to comment on that though (and I might not ever), so don't take my silence on that as either approval or disapproval.

[jeremyn]

f254015 takes care of my case-insensitivity and "main_frame" concerns, thanks!

[Hainish]

@jeremyn

• It would be nice if the ruleset list were updated in real-time, meaning if you have the HTTPS Everywhere UI open while the page is loading, you see the rulesets update while you are looking at the UI. It doesn't do that currently either, so this would be an improvement above-and-beyond the changes you're making. This is of course possible for JavaScript on a normal webpage but maybe it's not possible for extensions, or maybe it would require a ton of extra work.

This would make it difficult to deactivate a ruleset in the list if resources are still being loaded. Often it will take a while for all resources to be loaded, and if the list is shifting while this is happening it will make it hard to click on the corresponding check box. This problem would be exacerbated by the fact that rulesets are now alphabetical rather than listed in the order they are requested, since previously the checkboxes were be added on to the end, which would not interfere with the position of existing checkboxes.

I would leave off this change for now. We can deal with it separately in the future, but for this PR it seems like an orthogonal change.

[Hainish]

I wonder if this is strictly necessary. After all, https://github.com/EFForg/https-everywhere/pull/14678/files#diff-81d05cf2685349d7bbbe9f394bd74506R188 will simply override this value when the main_frame is loaded.

[cschanaj]

this will be necessary when there is no applicable ruleset applied to the main_frame but at least one ruleset applied to the page resources; this is typical when a HTTP-only site serving third-parties HTTPS content.

[jeremyn]

@Hainish You make a good point about the difficulty of interacting with the checkboxes while the list is being loaded if the items are being shuffled around. Maybe there is some middle ground between the current approach of doing nothing, and making it fully real-time. In any case I agree it is unrelated to the change of making the rulesets alphabetical, and I'm okay with not requiring related changes in this particular pull request.

[Hainish]

@cschanaj I'm ready to merge, pending feedback on #14678 (comment)

[Hainish]

Thanks, @cschanaj. This looks great!