Remove filter order randomization

[mjethani]

Background

In #4067 the order of all the filters was ​randomized. This was done mainly for element hiding so the website could ostensibly not read the "filter IDs" on Firefox (I have never understood this).

Anyway, in #6562 we ​stopped using filter keys for element hiding filters. This doesn't change the fact that the order in which the CSS selectors appear is deterministic (insertion order), which means the randomization in lib/filterListener.js would still make sense. Also, some Firefox-specific functionality for element hiding was removed in #4140.

Given that we now use user style sheets on Firefox (#5090) and the website has no way of reading this style sheet, it should be safe to remove the randomization, whatever its original purpose.

What to change

In lib/filterListener.js, add filters in the same order in which they occur in the subscription, without any randomization.

Hints for testers

Make sure of the following:

1. When a subscription is added, all the filters in the subscription get added and become active (i.e. if there's a filter ##.foo then it should hide the element <div class="foo">Ad</div> on the page)
2. When a subscription is disabled and then reenabled, all the filters in the subscription should become active again
3. When a subscription is updated, any new filters added to the subscription should become active. e.g. let's say a subscription is loaded remotely from https://example.com/foo.txt and it contains ##.foo at first, then you add the filter ##.bar to it and then manually update the subscription on the options page; the new filter should hide the element <div class="bar">Ad</div> on the page (after refreshing the page of course)
4. Any user-defined filter that is added should become active, i.e. it should work on the page

[mjethani]

What do you think, Dave?

[kzar]

Sounds OK to me, Manish, but I was not the mastermind behind the randomisation. The best person to ask would be Wladimir, alternatively I recommend having a look back through the commit history of the adblockplus repository for clues about why the randomisation was necessary.

[mjethani]

Thanks, Dave. I asked you because you ​reviewed the change, I figured you'd have some background (but you've probably forgotten because it was a long time ago).

Even if I were to go through the entire commit history, I would not really understand why this randomization was done. But I can try.

On the other hand, given how things work now (with user style sheets and everything), I can't think of any reason for the filter order to be randomized. If no one here has any ideas, let's just go ahead and simplify this code.

[kzar]

Yea, Manish, I remember reviewing that one.

I checked the code worked like he specified in the issue, but I didn't fully understand why the randomisation was necessary. Or, perhaps I asked him in IRC at the time, rather than in the code review, I can't remember! Like you say it was some time ago!

To understand why the randomisation was necessary in the first place we'd have to look through the adblockplus history, or ask Wladimir himself. The change you linked was not where the randomisation was originally introduced, but rather a necessary change for the getSelectorsByDomain optimisations I was working on at the time.

Sorry I couldn't be more help, FWIW going ahead with this sounds OK to me.

[sebastian]

I don't get all the details together either, but the randomization had something to do with how information of user stylesheets got leaked in legacy Gecko extensions. It should no longer be necessary for Web Extensions.

[abpbot]

A commit referencing this issue has landed:
​Issue 7178 - Remove filter order randomization

[abpbot]

A commit referencing this issue has landed:
​Issue 7178 - Remove filter order randomization