Applied changes from emailed code review

lib/rules.js

 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/FileUtils.jsm");
 
 let {Prefs} = require("prefs");
 
 let RULES_VERSION = 2;
+
+let CUSTOM_RULE_PRIORITY = 0x7FFFFFFF;
 
 let rules = {expressions: []};
 
 loadRules();
 
 // Make first attempt to update rules after five minutes
 let updateTimer = null;
 updateTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
 updateTimer.initWithCallback(onTimer, 1000 * 60 * 5, Ci.nsITimer.TYPE_REPEATING_SLACK);
 onShutdown.add(function() updateTimer.cancel());
@@ skipping 18 matching lines @@
   request.addEventListener("load", function()
   {
     try
     {
       // Remove comments from the file if any
       let data = JSON.parse(request.responseText.replace(/^\s*\/\/.*/mg, ""));
       if (ignoreVersion || data.version == RULES_VERSION)
       {
         rules = data;
         callback(true);
+
+        // Add user-defined rules after calling the callback - if the callback
+        // saves the rules then the custom rules won't be included.
+        addCustomRules();
       }
       else
         callback(false);
     }
     catch (e)
     {
       Cu.reportError(e);
       callback(false);
     }
   }, false);
@@ skipping 13 matching lines @@
     callback(false);
   }
 }
 
 function getRuleFile()
 {
   let result = FileUtils.getFile("ProfD", ["url-fixer-rules.json"]);
 
   getRuleFile = function() result;
   return getRuleFile();
+}
+
+function addCustomRules()
+{
+  for (let domain in Prefs.whitelist)
+    onWhitelistEntryAdded(domain);
+}
+
+function onWhitelistEntryAdded(domain)
+{
+  let reverse = domain.split("").reverse().join("");
+  addSuffix(rules.domain, reverse, CUSTOM_RULE_PRIORITY);
+}
+exports.onWhitelistEntryAdded = onWhitelistEntryAdded;
+
+function onWhitelistEntryRemoved(domain)
+{
+  let reverse = domain.split("").reverse().join("");
+  removeSuffix(rules.domain, reverse, CUSTOM_RULE_PRIORITY);
+}
+exports.onWhitelistEntryRemoved = onWhitelistEntryRemoved;
+
+function addSuffix(tree, suffix, priority)
+{
+  if (suffix.length == 0)
+  {
+    // We are at the last character, just put our priority here
+    tree[""] = " " + priority;
+    return;
+  }
+
+  let c = suffix[0];
+  if (c in tree)
+  {
+    let existing = tree[c];
+    if (typeof existing == "string")
+    {
+      // Single choice for this suffix, maybe the same entry?
+      if (existing.substr(0, suffix.length - 1) == suffix.substr(1) && existing[suffix.length - 1] == " ")
+      {
+        // Same entry, simply replace it by new priority
+        tree[c] = suffix.substr(1) + " " + priority;
+      }
+      else
+      {
+        // Different entry, need to add a new branching point and go deeper
+        if (existing[0] == " ")
+          tree[c] = {"": existing};
+        else
+        {
+          tree[c] = {};
+          tree[c][existing[0]] = existing.substr(1);
+        }
+        addSuffix(tree[c], suffix.substr(1), priority);
+      }
+    }
+    else
+    {
+      // Multiple choices for this suffix - go deeper
+      addSuffix(existing, suffix.substr(1), priority);
+    }
+  }
+  else
+  {
+    // No existing entry yet, just add ours
+    tree[c] = suffix.substr(1) + " " + priority;
+  }
+}
+
+function removeSuffix(tree, suffix, priority)
+{
+  if (suffix.length == 0)
+  {
+    // We are at the last character, check whether there is an entry with
+    // matching priority
+    if ("" in tree && tree[""] == " " + priority)
+      delete tree[""];
+    return;
+  }
+
+  let c = suffix[0];
+  if (!(c in tree))
+    return;
+
+  if (typeof tree[c] == "string")
+  {
+    // Single entry - check whether it is the right one
+    if (tree[c] == suffix.substr(1) + " " + priority)
+      delete tree[c];
+  }
+  else
+  {
+    // Multiple entries, need to go deeper
+    removeSuffix(tree[c], suffix.substr(1), priority);
+  }
 }
 
 function onTimer()
 {
   // Next check in 1 hour
   updateTimer.delay = 1000 * 60 * 60;
 
   // Only download rules every three days
   let nextUpdate = Prefs.lastRuleUpdate + 60 * 60 * 24 * 3;
   if (nextUpdate > Date.now() / 1000)
     return;
 
   loadRulesFrom("http://urlfixer.org/download/rules.json?version=" + RULES_VERSION, false, function(success)
   {
     if (success)
     {
       rules.timestamp = Date.now();
-
-      var dynRules = require("updateRules").updateRules();
-      for(var i in dynRules)
-      {
-        rules[i] = dynRules[i];
-      }
 
       try
       {
         // Save the rules to file.
         let rulesText = JSON.stringify(rules);
         let fileStream = FileUtils.openSafeFileOutputStream(getRuleFile());
         let stream = Cc["@mozilla.org/intl/converter-output-stream;1"].createInstance(Ci.nsIConverterOutputStream);
         stream.init(fileStream, "UTF-8", 16384, Ci.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER);
         stream.writeString(rulesText);
         stream.flush();
@@ skipping 27 matching lines @@
   // Apply user's custom changes first
   let customRules = Prefs.custom_replace;
   for (let searchString in customRules)
   {
     let replacement = customRules[searchString];
     searchString = searchString.toLowerCase();
     if (/^re:+/.test(searchString))
       domain = domain.replace(new RegExp(RegExp.rightContext, "g"), replacement);
     else
       domain = domain.replace(searchString, replacement);
-  }
-  
-  // Apply user's whitelist
-  let whitelist = Prefs.whitelist;
-  if (whitelist.hasOwnProperty(domain) || /^www\./.test(domain) && whitelist.hasOwnProperty(domain.substr(4)))
-  {
-    return domain;
   }

[Wladimir Palant, 2012/09/21 14:40:25]

That's not what should be done with the whitelist. Suppose you have "foo.com" in
the whitelist, should "example.foo.com" still be corrected into
"example.food.com"? I don't think so. So whitelist needs to be merged with the
server-supplied rules. This will have two advantages over the approach here:

a) Subdomains of a whitelisted domain won't be corrected.
b) Corrections towards whitelisted domains will be possible. E.g. if somebody
adds "mygreatwebsite.com" and mistypes it as "mygretwebsite.com" later - we
should correct that.

 
   // Now apply our rules on the domain name
   for (let i = 0, l = rules.expressions.length; i < l; i++)
     domain = applyExpression(domain, rules.expressions[i]);
 
   // Find similar known domains, test domains without the www. prefix
   if (domain.substr(0, 4) == "www.")
     domain = "www." + getBestMatch(domain.substr(4), rules.domain, 1, ".");
   else
     domain = getBestMatch(domain, rules.domain, 1, ".");
@@ skipping 142 matching lines @@
       bestSuggestionDistance = distance;
       bestSuggestionMatched = matchedLen;
       bestSuggestionPriority = priority;
     }
   }
   if (bestSuggestion)
     return input.substr(0, input.length - bestSuggestionMatched) + bestSuggestion;
   else
     return input;
 }