Noissue - Trim spaces around element hiding emulation selectors

test/filterClasses.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 414 matching lines @@
 
   // Comment filters
   test.equal(Filter.normalize("   !  fo  o##  bar   "),
              "!  fo  o##  bar");
 
   // Element hiding filters
   test.equal(Filter.normalize("   domain.c  om## # sele ctor   "),
              "domain.com### sele ctor");
 
   // Element hiding emulation filters
   test.equal(Filter.normalize("   domain.c  om#?# # sele ctor   "),

[kzar, 2018/04/11 11:29:49]

What about a test for something like "domain.com# ?#selector"?

[Manish Jethani, 2018/04/11 11:50:19]

Interesting, the test would pass for "# ?#" but not for "# ##" and "# @##",
because Filter.elemhideRegExp does not include a "?". I've updated it now.

Specifically "domain.com# ?##  selector  " now normalizes to
"domain.com#?##selector" (note: no spaces) because elemhideRegExp no longer
matches. This is consistent with "domain.com# ##  selector  " and "domain.com#
@##  selector  ". I'm not sure if it is right though.

[kzar, 2018/04/11 12:04:09]

Well it's good that we're being consistent now, but I agree that maybe we're
being consistently wrong! Really if we're to be as thorough with the
normalisation of whitespace with #?# etc filters as we were with $csp filters we
should handle this case too... but I wonder at what cost that will come, or if
there's any benefit to it.

[Manish Jethani, 2018/04/11 17:34:54]

OK, I thought about this a little bit. elemhideRegExp does not accept spaces
within the separator (i.e. "# ?#" is not a valid separator). We could allow
spaces, but that would be a change in behavior, not sure we want that (at least
not as part of this patch).

Since spaces are not valid, the normalize function will not treat it as an
element hiding filter; this is the current behavior, we should not change it as
part of this patch.

Therefore the test you suggested is not valid, so let's not include that one.

The rest of this change is still valid though, we still can trim spaces around
the selector for "#?#" filters.

              "domain.com#?## sele ctor");
 
+  // Incorrect syntax: the separator "#?#" cannot contain spaces; treated as a
+  // regular filter instead
+  test.equal(Filter.normalize("   domain.c  om# ?#. sele ctor   "),
+             "domain.com#?#.selector");
+  // Incorrect syntax: the separator "#?#" cannot contain spaces; treated as an
+  // element hiding filter instead, because the "##" following the "?" is taken
+  // to be the separator instead
+  test.equal(Filter.normalize("   domain.c  om# ?##sele ctor   "),
+             "domain.com#?##sele ctor");
+
   // Element hiding exception filters
   test.equal(Filter.normalize("   domain.c  om#@# # sele ctor   "),
              "domain.com#@## sele ctor");
+
+  // Incorrect syntax: the separator "#@#" cannot contain spaces; treated as a
+  // regular filter instead (not an element hiding filter either!), because
+  // unlike the case with "# ?##" the "##" following the "@" is not considered
+  // to be a separator
+  test.equal(Filter.normalize("   domain.c  om# @## sele ctor   "),
+             "domain.com#@##selector");
 
   // Regular filters
   let normalized = Filter.normalize(
     "    b$l     a$sitekey=  foo  ,domain= do main.com |foo   .com,c sp= c   s p  "
   );
   test.equal(
     normalized,
     "b$la$sitekey=foo,domain=domain.com|foo.com,csp=c s p"
   );
   compareFilter(
@@ skipping 25 matching lines @@
              "foo$csp=bar,$csp=c s p");
   test.equal(Filter.normalize(" f o   o   $      bar   $csp=ba r"),
              "foo$bar$csp=ba r");
   test.equal(Filter.normalize("f    $    o    $    o    $    csp=f o o "),
              "f$o$o$csp=f o o");
   test.equal(Filter.normalize("/foo$/$ csp = script-src  http://example.com/?$1=1&$2=2&$3=3"),
              "/foo$/$csp=script-src http://example.com/?$1=1&$2=2&$3=3");
 
   test.done();
 };