Applied changes from emailed code review

lib/typoFixer.js

Index: lib/typoFixer.js
===================================================================
--- a/lib/typoFixer.js
+++ b/lib/typoFixer.js
@@ -143,8 +143,7 @@
 function correctURL(window, value)
 {
   let hasCorrection = false;
-
-  // Trim it
+  
   value = value.trim();
   if (value.length == 0)
     return null;
@@ -178,9 +177,9 @@
     return null;
 
   // Check manually entered corrections
-  if (Prefs.custom_replace[value])
+  if (Prefs.custom_replace.hasOwnProperty(value) && Prefs.custom_replace[value])
     return Prefs.custom_replace[value];
-
+  
   let [prefix, domain, suffix] = parseURL(value);
   if (!domain)
     return null;
@@ -231,54 +230,38 @@
     return null;
 
   // Show infobar to inform and ask about correction
-  let browser = appIntegration.getBrowser(window);
-  let infobar = browser.getNotificationBox();
-  let notif = infobar.getNotificationWithValue("url-fixer-infobar-askafter");
+  let [message, yes, no, cancel] = getInfobarTexts();

[Wladimir Palant, 2012/09/28 09:29:34]

You don't use a Cancel button - please remove the variable and the corresponding
strings.

+  message = message.replace(/\?1\?/g, prefix+domain);
+  let buttons = [
+    {
+      label:      yes,
+      accessKey:  null,
+      callback:   function()
+      {
+        // Yes: Do nothing
+      }
+    },
+    {
+      label:      no,
+      accessKey:  null,
+      callback:   function()
+      {
+        // No: Add to list of corrections (ignore)
+        if (/^www\./.test(value))
+        {
+          value = value.substr(4);
+        }
+        Prefs.whitelist[value] = value;

[Wladimir Palant, 2012/09/28 09:29:34]

Comments from previous review not addressed here - one of my changes replaced
the code here however.

+        Prefs.whitelist = JSON.parse(JSON.stringify(Prefs.whitelist));
 
-  let [message, yes, no, cancel] = getAskAfterDialogTexts();
-  message = message.replace(/\?1\?/g, prefix+domain);
+        require("appIntegration").loadURI(value);
+        processFalsePositive(oldDomain, domain);
+      }
+    }
+  ];
+  require("appIntegration").openInfobar(window, "url-fixer-infobar-askafter", message, buttons, 1);

[Wladimir Palant, 2012/09/28 09:29:34]

It isn't obvious why we pass 1 for persistence here, I had to look it up myself.
Reason is that we are about to load a different page which will change location
- so the message needs to survive one location change. Maybe add a comment
explaining that?

 
-  if (notif)
-  {
-    notif.label = message;
-  }
-  else
-  {
-    let buttons = [
-      {
-        label:      yes,
-        accessKey:  null,
-        callback:   function()
-        {
-          // Yes: Do nothing
-        }
-      },
-      {
-        label:      no,
-        accessKey:  null,
-        callback:   function()
-        {
-          // No: Add to list of corrections (ignore)
-          // TODO: maybe find more appropriate place to store this information
-          Prefs.custom_replace[value] = value;
-          Prefs.custom_replace = JSON.parse(JSON.stringify(Prefs.custom_replace));
-
-          browser.loadURI(value);
-          processFalsePositive(oldDomain, domain);
-        }
-      }
-    ];
-    notif = infobar.appendNotification(
-      message,
-      "url-fixer-infobar-askafter",
-      require("info").addonRoot + "icon64.png",
-      infobar.PRIORITY_INFO_LOW,
-      buttons
-    );
-    notif.persistence = 1;
-  }
-
-  require("survey").incrementCorrectionsCounter(window);
+  require("survey").incrementCorrectionsCounter();
 
   // Consider the correction a second typed domain
   if (!isIPAddress(domain))
@@ -289,8 +272,9 @@
 
 let stringBundle = null;
 
-function getAskAfterDialogTexts()
+function getInfobarTexts()
 {
+  // Randomize URI to work around bug 719376
   if (!stringBundle)
     stringBundle = Services.strings.createBundle("chrome://url-fixer/locale/locale.properties?" + Math.random());
   let result = [
@@ -300,6 +284,6 @@
     stringBundle.GetStringFromName("urlfixer.cancel")
   ];
 
-  getAskAfterDialogTexts = function() result;
-  return getAskAfterDialogTexts();
+  getInfobarTexts = function() result;
+  return getInfobarTexts();
 }