Issue 2394 - Added unit tests for CSS property filters

chrome/content/tests/filterClasses.js

 (function()
 {
   module("Filter classes", {setup: prepareFilterComponents, teardown: restoreFilterComponents});
 
   function serializeFilter(filter)
   {
     // Filter serialization only writes out essential properties, need to do a full serialization here
     let result = [];
     result.push("text=" + filter.text);
     if (filter instanceof InvalidFilter)
@@ skipping 40 matching lines @@
         {
           result.push("type=whitelist");
         }
       }
       else if (filter instanceof ElemHideBase)
       {
         if (filter instanceof ElemHideFilter)
           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);
       }
     }
     return result;
   }
 
   function addDefaults(expected)
   {
     let type = null;
     let hasProperty = {};
     for (let entry of expected)
     {
       if (/^type=(.*)/.test(entry))
         type = RegExp.$1;
       else if (/^(\w+)/.test(entry))
         hasProperty[RegExp.$1] = true;
     }
 
     function addProperty(prop, value)
     {
       if (!(prop in hasProperty))
         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");
       addProperty("hitCount", "0");
     }
     if (type == "whitelist" || type == "filterlist")
     {
       addProperty("contentType", 0x7FFFFFFF & ~(RegExpFilter.typeMap.DOCUMENT | RegExpFilter.typeMap.ELEMHIDE | RegExpFilter.typeMap.POPUP));
       addProperty("matchCase", "false");
       addProperty("thirdParty", "null");
       addProperty("domains", "");
       addProperty("sitekeys", "");
     }
     if (type == "filterlist")
     {
       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)
   {
     addDefaults(expected);
 
     let filter = Filter.fromText(text);
     if (postInit)
       postInit(filter)
     let result = serializeFilter(filter);
@@ skipping 26 matching lines @@
     equal(typeof Filter, "function", "typeof Filter");
     equal(typeof InvalidFilter, "function", "typeof InvalidFilter");
     equal(typeof CommentFilter, "function", "typeof CommentFilter");
     equal(typeof ActiveFilter, "function", "typeof ActiveFilter");
     equal(typeof RegExpFilter, "function", "typeof RegExpFilter");
     equal(typeof BlockingFilter, "function", "typeof BlockingFilter");
     equal(typeof WhitelistFilter, "function", "typeof WhitelistFilter");
     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()
   {
     compareFilter("!asdf", ["type=comment", "text=!asdf"]);
     compareFilter("!foo#bar", ["type=comment", "text=!foo#bar"]);
     compareFilter("!foo##bar", ["type=comment", "text=!foo##bar"]);
   });
 
   test("Invalid filters", function()
   {
     compareFilter("/??/", ["type=invalid", "text=/??/", "hasReason"]);
 
     compareFilter("#dd(asd)(ddd)", ["type=invalid", "text=#dd(asd)(ddd)", "hasReason"]);
     {
       let result = Filter.fromText("#dd(asd)(ddd)").reason;
       equal(result, Utils.getString("filter_elemhide_duplicate_id"), "#dd(asd)(ddd).reason");
     }
 
     compareFilter("#*", ["type=invalid", "text=#*", "hasReason"]);
     {
       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()
   {
     compareFilter("blabla", ["type=filterlist", "text=blabla", "regexp=blabla"]);
     compareFilter("blabla_default", ["type=filterlist", "text=blabla_default", "regexp=blabla_default"], function(filter)
     {
       filter.disabled = false;
       filter.hitCount = 0;
       filter.lastHit = 0;
@@ skipping 58 matching lines @@
     compareFilter("#ddd(fff)", ["type=elemhide", "text=#ddd(fff)", "selector=ddd.fff,ddd#fff"]);
     compareFilter("#ddd(foo=bar)(foo2^=bar2)(foo3*=bar3)(foo4$=bar4)", ["type=elemhide", "text=#ddd(foo=bar)(foo2^=bar2)(foo3*=bar3)(foo4$=bar4)", 'selector=ddd[foo="bar"][foo2^="bar2"][foo3*="bar3"][foo4$="bar4"]']);
     compareFilter("#ddd(fff)(foo=bar)", ["type=elemhide", "text=#ddd(fff)(foo=bar)", 'selector=ddd.fff[foo="bar"],ddd#fff[foo="bar"]']);
     compareFilter("#*(fff)", ["type=elemhide", "text=#*(fff)", "selector=.fff,#fff"]);
     compareFilter("#*(foo=bar)", ["type=elemhide", "text=#*(foo=bar)", 'selector=[foo="bar"]']);
     compareFilter("##body > div:first-child", ["type=elemhide", "text=##body > div:first-child", "selector=body > div:first-child"]);
     compareFilter("foo#ddd", ["type=elemhide", "text=foo#ddd", "selectorDomain=foo", "selector=ddd", "domains=FOO"]);
     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()
   {
     compareFilter("#@ddd", ["type=elemhideexception", "text=#@ddd", "selector=ddd"]);
     compareFilter("#@ddd(fff)", ["type=elemhideexception", "text=#@ddd(fff)", "selector=ddd.fff,ddd#fff"]);
     compareFilter("#@ddd(foo=bar)(foo2^=bar2)(foo3*=bar3)(foo4$=bar4)", ["type=elemhideexception", "text=#@ddd(foo=bar)(foo2^=bar2)(foo3*=bar3)(foo4$=bar4)", 'selector=ddd[foo="bar"][foo2^="bar2"][foo3*="bar3"][foo4$="bar4"]']);
     compareFilter("#@ddd(fff)(foo=bar)", ["type=elemhideexception", "text=#@ddd(fff)(foo=bar)", 'selector=ddd.fff[foo="bar"],ddd#fff[foo="bar"]']);
     compareFilter("#@*(fff)", ["type=elemhideexception", "text=#@*(fff)", "selector=.fff,#fff"]);
     compareFilter("#@*(foo=bar)", ["type=elemhideexception", "text=#@*(foo=bar)", 'selector=[foo="bar"]']);
     compareFilter("#@#body > div:first-child", ["type=elemhideexception", "text=#@#body > div:first-child", "selector=body > div:first-child"]);
     compareFilter("foo#@ddd", ["type=elemhideexception", "text=foo#@ddd", "selectorDomain=foo", "selector=ddd", "domains=FOO"]);
     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="]);
   });
 })();