Adding ABP core modules to ABP/Opera

lib/matcher.js

+/*
+ * This Source Code is subject to the terms of the Mozilla Public License
+ * version 2.0 (the "License"). You can obtain a copy of the License at
+ * http://mozilla.org/MPL/2.0/.
+ */
+
+//
+// This mimicks the API of the Matcher module in ABP/Firefox but users Opera's
+// URLFilter API.
+//
+
+(function()
+{
+  var WhitelistFilter = null;
+  var RegExpFilter = null;
+  var resourceTypes = [
+      "DOCUMENT", "FONT", "IMAGE", "MEDIA", "OBJECT", "OBJECT_SUBREQUEST",
+      "OTHER", "SCRIPT", "STYLESHEET", "SUBDOCUMENT", "XMLHTTPREQUEST"
+  ];
+
+  require.scopes.matcher =
+  {
+    defaultMatcher:
+    {
+      _rules: {},
+
+      _generateRule: function(filter)

[Felix Dahlke, 2012/10/10 12:11:22]

I'd love it if the code segments below comments ("Convert anchors, ...",
"Translate type options" etc.) would be extracted to functions. Probably just a
matter of taste, but I find small functions with descriptive names easier to
understand than larger ones with comments. I've also given Andrey that feedback
a lot - it's just my opinion, nothing I insist on.

+      {
+        if (!WhitelistFilter)
+        {
+          WhitelistFilter = require("filterClasses").WhitelistFilter;
+          RegExpFilter = require("filterClasses").RegExpFilter;
+        }
+
+        var rule = {
+          type: filter instanceof WhitelistFilter ? "allow" : "block",
+          text: filter.regexpSource,
+          options: {}
+        };
+
+        // Convert anchors, Opera requires explicit *
+
+        // |foo => foo
+        // foo => *foo
+        // *foo and ||foo stay unchanged
+        if (rule.text.substr(0, 2) != "||")
+        {
+          if (rule.text.substr(0, 1) == "|")

[Felix Dahlke, 2012/10/10 12:11:22]

This value is used twice, so how about storing it in a variable? I'd also prefer
[0] to .substr(0, 1):

var firstChar = rule.text[0];
if (firstChar == "|")
...

Only a readability aid, we can of course rely on the idempotency of [] and
substr().

+            rule.text = rule.text.slice(1);
+          else if (rule.text.substr(0, 1) != "*")
+            rule.text = "*" + rule.text;
+        }
+
+        // foo| => foo
+        // foo => foo*
+        // foo* stays unchanged
+        if (rule.text.substr(-1) == "|")

[Felix Dahlke, 2012/10/10 12:11:22]

As above, how about storing this in a variable lastChar?

+          rule.text = rule.text.slice(0, -1);
+        else if (rule.text.substr(-1) != "*")
+          rule.text = rule.text + "*";
+
+        // Translate type options
+        var resources = 0;
+        var urlfilter = opera.extension.urlfilter;
+        for (var i = 0; i < resourceTypes.length; i++)
+        {
+          var type = resourceTypes[i];
+          if (filter.contentType & RegExpFilter.typeMap[type])
+            resources = resources | urlfilter["RESOURCE_" + type];

[Felix Dahlke, 2012/10/10 12:11:22]

How about:
resources |= urlfilter["RESOURCE_" + type];

[Wladimir Palant, 2012/10/11 09:36:26]

I thought that JavaScript doesn't support |= but it seems that I was mistaken -
looks like I was thinking about ||= that is indeed not supported.

+        }
+        rule.options.resources = resources;
+
+        // Translate domain options
+        if (filter.domainSource)
+        {
+          var domains = filter.domainSource.split(filter.domainSeparator);
+          for (var i = 0; i < domains.length; i++)
+          {
+            var domain = domains[i];
+            if (domain == "")
+              continue;
+
+            var type = "includeDomains";
+            if (domain[0] == "~")
+            {
+              type = "excludeDomains";
+              domain = domain.substr(1);
+            }
+            if (!(type in rule.options))
+              rule.options[type] = [];
+            rule.options[type].push(domain);
+          }
+        }
+
+        // Translate third-party option
+        if (filter.thirdParty !== null)
+          rule.options.thirdParty = filter.thirdParty;
+
+        return rule;
+      },
+
+      add: function(filter)
+      {
+        if (filter.text in this._rules)
+          return;
+
+        var rule = this._generateRule(filter);
+        if (filter.text == "http://")

[Felix Dahlke, 2012/10/10 12:11:22]

Why just check for the string "http://" here? Wouldn't "foobar" or anything
that's not a valid filter be equally worth reporting?

[Wladimir Palant, 2012/10/11 09:36:26]

This is just debugging code I forgot :)

+          Cu.reportError(JSON.stringify(rule));
+        opera.extension.urlfilter[rule.type].add(rule.text, rule.options);
+        this._rules[filter.text] = rule;
+      },
+
+      remove: function(filter)
+      {
+        if (!(filter.text in this._rules))
+          return;
+
+        var rule = this._rules[filter.text];
+        opera.extension.urlfilter[rule.type].remove(rule.text);
+        delete this._rules[filter.text];
+      },
+
+      clear: function(filter)
+      {
+        for (var text in this._rules)
+        {
+          var rule = this._rules[text];
+          opera.extension.urlfilter[rule.type].remove(rule);

[Felix Dahlke, 2012/10/10 12:11:22]

Shouldn't this be "remove(text)"?

+        }
+        this._rules = {};
+      }
+    }
+  };
+})();