Noissue - Simplify unconditional selector lookup

Noissue - Simplify unconditional selector lookup

Depends on changes in this review: https://codereview.adblockplus.org/29349187/

[kzar, 2016-09-20 09:23:57]

Patch Set 1

[Wladimir Palant, 2016-09-20 10:43:07]

NOT LGTM

I see now why you considered that change simple. However, we definitely have
unconditional filters that aren't identical yet share the same selector (##foo
and ,,##foo are the ones that I found, there might be other scenarios as well
however). This change will introduce correctness issues, e.g. if both ##foo and
,,##foo are added and then one of them is removed.

It's possible to implement this correctly, it would require ##foo and ,,##foo
(and whatever other variants there might be) to be treated as the same filter so
that FilterListener won't add both of them. With our current codebase only
performing filter normalization on demand (unlike the work in progress based on
Emscripten), I don't really see an easy way to implement this however.

[kzar, 2016-09-20 15:10:56]

On 2016/09/20 10:43:07, Wladimir Palant wrote:
> ...However, we definitely have
> unconditional filters that aren't identical yet share the same selector (##foo
> and ,,##foo are the ones that I found, there might be other scenarios as well
> however). This change will introduce correctness issues, e.g. if both ##foo
and
> ,,##foo are added and then one of them is removed.

Until the recent change filters like ",,##foo" would have resulted in an
exception. But even now that it's even possible I don't think the complexity of
tracking multiple filter keys for unconditional selectors is worth handling such
a crazy edge case.

[Wladimir Palant, 2016-09-20 16:13:27]

On 2016/09/20 15:10:56, kzar wrote:
> Until the recent change filters like ",,##foo" would have resulted in an
> exception. But even now that it's even possible I don't think the complexity
of
> tracking multiple filter keys for unconditional selectors is worth handling
such
> a crazy edge case.

Well, we can officially declare ",,##foo" invalid (as in, resulting in an
InvalidFilter instance) - then we only need to verify that no other scenario
will result in different unconditional filters for same selector and we can do
this without sacrificing consistency.

[kzar, 2016-09-20 17:43:27]

On 2016/09/20 16:13:27, Wladimir Palant wrote:
> Well, we can officially declare ",,##foo" invalid (as in, resulting in an
> InvalidFilter instance) - then we only need to verify that no other scenario
> will result in different unconditional filters for same selector and we can do
> this without sacrificing consistency.

Sounds good to me, I've opened an issue for the first part here
https://issues.adblockplus.org/ticket/4450