Issue 2394 - Added unit tests for CSS property filters

chrome/content/tests/filterClasses.js

Index: chrome/content/tests/filterClasses.js
===================================================================
--- a/chrome/content/tests/filterClasses.js
+++ b/chrome/content/tests/filterClasses.js
@@ -58,6 +58,13 @@
           result.push("type=elemhide");
         else if (filter instanceof ElemHideException)
           result.push("type=elemhideexception");
+        else if (filter instanceof CSSPropertyFilter)
+        {
+          result.push("type=cssrule");
+          result.push("prefix=" + (filter.selectorPrefix || ""));
+          result.push("regexp=" + filter.regexpString);
+          result.push("suffix=" + (filter.selectorSuffix || ""));
+        }
 
         result.push("selectorDomain=" + (filter.selectorDomain || ""));
         result.push("selector=" + filter.selector);
@@ -84,7 +91,7 @@
         expected.push(prop + "=" + value);
     }
 
-    if (type == "whitelist" || type == "filterlist" || type == "elemhide" || type == "elemhideexception")
+    if (type == "whitelist" || type == "filterlist" || type == "elemhide" || type == "elemhideexception" || type == "cssrule")
     {
       addProperty("disabled", "false");
       addProperty("lastHit", "0");
@@ -102,11 +109,17 @@
     {
       addProperty("collapse", "null");
     }
-    if (type == "elemhide" || type == "elemhideexception")
+    if (type == "elemhide" || type == "elemhideexception" || type == "cssrule")
     {
       addProperty("selectorDomain", "");
       addProperty("domains", "");
     }
+    if (type == "cssrule")
+    {
+      addProperty("regexp", "");
+      addProperty("prefix", "");
+      addProperty("suffix", "");
+    }
   }
 
   function compareFilter(text, expected, postInit)
@@ -153,6 +166,7 @@
     equal(typeof ElemHideBase, "function", "typeof ElemHideBase");
     equal(typeof ElemHideFilter, "function", "typeof ElemHideFilter");
     equal(typeof ElemHideException, "function", "typeof ElemHideException");
+    equal(typeof CSSPropertyFilter, "function", "typeof CSSPropertyFilter");
   });
 
   test("Comments", function()
@@ -177,6 +191,19 @@
       let result = Filter.fromText("#*").reason;
       equal(result, Utils.getString("filter_elemhide_nocriteria"), "#*.reason");
     }
+
+    function compareCSSRule(domains)
+    {
+      let filterText = domains + "##[-abp-properties='abc']";
+      compareFilter(filterText, ["type=invalid", "text=" + filterText, "hasReason"]);
+      let reason = Filter.fromText(filterText).reason;
+      equal(reason, Utils.getString("filter_cssproperty_nodomain"), filterText + ".reason");
+    }
+    compareCSSRule("");
+    compareCSSRule("~foo.com");
+    compareCSSRule("~foo.com,~bar.com");
+    compareCSSRule("foo");

[Wladimir Palant, 2015/11/10 10:49:03]

Why is foo##[-abp-properties='abc'] invalid?

[Thomas Greiner, 2015/12/03 12:55:20]

Considering domains with only one part (e.g. "com", "de") as generic was
suggested by Sebastian in the filter class review (see
https://codereview.adblockplus.org/5730585574113280/diff/5629499534213120/lib...).

+    compareCSSRule("~foo.com,bar");
   });
 
   test("Filters with state", function()
@@ -255,6 +282,7 @@
     compareFilter("foo,bar#ddd", ["type=elemhide", "text=foo,bar#ddd", "selectorDomain=foo,bar", "selector=ddd", "domains=BAR|FOO"]);
     compareFilter("foo,~bar#ddd", ["type=elemhide", "text=foo,~bar#ddd", "selectorDomain=foo", "selector=ddd", "domains=FOO|~BAR"]);
     compareFilter("foo,~baz,bar#ddd", ["type=elemhide", "text=foo,~baz,bar#ddd", "selectorDomain=foo,bar", "selector=ddd", "domains=BAR|FOO|~BAZ"]);
+    compareFilter("##[-abp-properties='']", ["type=elemhide", "text=##[-abp-properties='']", "selector=[-abp-properties='']"]);

[Wladimir Palant, 2015/11/10 10:49:03]

Wouldn't it make sense to have this filter in the "CSS property filters" test
since you've added one?

[Thomas Greiner, 2015/12/03 12:55:20]

Done.

   });
 
   test("Element hiding exceptions", function()
@@ -270,5 +298,18 @@
     compareFilter("foo,bar#@ddd", ["type=elemhideexception", "text=foo,bar#@ddd", "selectorDomain=foo,bar", "selector=ddd", "domains=BAR|FOO"]);
     compareFilter("foo,~bar#@ddd", ["type=elemhideexception", "text=foo,~bar#@ddd", "selectorDomain=foo", "selector=ddd", "domains=FOO|~BAR"]);
     compareFilter("foo,~baz,bar#@ddd", ["type=elemhideexception", "text=foo,~baz,bar#@ddd", "selectorDomain=foo,bar", "selector=ddd", "domains=BAR|FOO|~BAZ"]);
+    compareFilter("foo.com#@#[-abp-properties='abc']", ["type=elemhideexception", "text=foo.com#@#[-abp-properties='abc']", "selectorDomain=foo.com", "selector=[-abp-properties='abc']", "domains=FOO.COM"]);

[Wladimir Palant, 2015/11/10 10:49:02]

Wouldn't it make sense to have this filter in the "CSS property filters" test
since you've added one?

[Thomas Greiner, 2015/12/03 12:55:21]

Done.

+  });
+
+  test("CSS property filters", function()
+  {
+    // Check valid domain combinations
+    compareFilter("foo.com##[-abp-properties='abc']", ["type=cssrule", "text=foo.com##[-abp-properties='abc']", "selectorDomain=foo.com", "selector=[-abp-properties='abc']", "domains=FOO.COM", "prefix=", "regexp=abc", "suffix="]);
+    compareFilter("foo.com,~bar.com##[-abp-properties='abc']", ["type=cssrule", "text=foo.com,~bar.com##[-abp-properties='abc']", "selectorDomain=foo.com", "selector=[-abp-properties='abc']", "domains=FOO.COM|~BAR.COM", "prefix=", "regexp=abc", "suffix="]);
+    compareFilter("foo.com,~bar##[-abp-properties='abc']", ["type=cssrule", "text=foo.com,~bar##[-abp-properties='abc']", "selectorDomain=foo.com", "selector=[-abp-properties='abc']", "domains=FOO.COM|~BAR", "prefix=", "regexp=abc", "suffix="]);
+    compareFilter("~foo.com,bar.com##[-abp-properties='abc']", ["type=cssrule", "text=~foo.com,bar.com##[-abp-properties='abc']", "selectorDomain=bar.com", "selector=[-abp-properties='abc']", "domains=BAR.COM|~FOO.COM", "prefix=", "regexp=abc", "suffix="]);

[Wladimir Palant, 2015/11/10 10:49:03]

You make prefix and suffix default to an empty string so specifying these in the
expected result it unnecessary.

[Thomas Greiner, 2015/12/03 12:55:21]

Done.

+
+    compareFilter("foo.com##aaa [-abp-properties='abc'] bbb", ["type=cssrule", "text=foo.com##aaa [-abp-properties='abc'] bbb", "selectorDomain=foo.com", "selector=aaa [-abp-properties='abc'] bbb", "domains=FOO.COM", "prefix=aaa ", "regexp=abc", "suffix= bbb"]);
+    compareFilter("foo.com##[-abp-properties='|background-image: url(data:*)']", ["type=cssrule", "text=foo.com##[-abp-properties='|background-image: url(data:*)']", "selectorDomain=foo.com", "selector=[-abp-properties='|background-image: url(data:*)']", "domains=FOO.COM", "prefix=", "regexp=^background\\-image\\:\\ url\\(data\\:.*\\)", "suffix="]);
   });
 })();