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 413 matching lines @@
              "ads");
 
   // 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");
+
+  // Element hiding exception filters
+  test.equal(Filter.normalize("   domain.c  om#@# # sele ctor   "),
+             "domain.com#@## sele ctor");
+
   // 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(
     test, normalized, [
@@ skipping 24 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();
 };