Applied changes from emailed code review

lib/survey.js

Index: lib/survey.js
===================================================================
--- a/lib/survey.js
+++ b/lib/survey.js
@@ -6,60 +6,28 @@
 
 let {Prefs} = require("prefs");
 
-let win = null;
 let surveyLang = null;
 let surveyUrl = "http://urlfixer.org"; //"http://adblockplus.org/usersurvey/index.php?sid=68316";
 
-let langData = {
-  en: {
-    title: "Tell us your opinion",
-    question: "We would like to ask you a few questions about URL Fixer to help us improve it. If you can spare 5 minutes please click the button below to take the survey.",
-    note: "This is a one-time message and will not appear again.",
-    accept: "Take the survey",
-    decline: "Maybe some other time"
-  },
-  de: {
-    title: "Sagen Sie uns Ihre Meinung",
-    question: "Wir w\xFCrden Ihnen gerne einige Fragen zu URL Fixer stellen, um es verbessern zu k\xF6nnen. Falls Sie gerade 5 Minuten haben, dr\xFCcken Sie bitte die Taste unten, um an der Nutzerumfrage teilzunehmen.",
-    note: "Das ist eine einmalige Nachricht, die nicht wieder erscheinen wird.",
-    accept: "An der Umfrage teilnehmen",
-    decline: "Vielleicht ein anderes Mal"
-  },
-  ru: {
-    title: decodeURIComponent("%D0%9F%D0%BE%D0%B4%D0%B5%D0%BB%D0%B8%D1%82%D0%B5%D1%81%D1%8C%20%D1%81%20%D0%BD%D0%B0%D0%BC%D0%B8%20%D1%81%D0%B2%D0%BE%D0%B8%D0%BC%20%D0%BC%D0%BD%D0%B5%D0%BD%D0%B8%D0%B5%D0%BC"),
-    question: decodeURIComponent("%D0%9C%D1%8B%20%D1%85%D0%BE%D1%82%D0%B5%D0%BB%D0%B8%20%D0%B1%D1%8B%20%D0%B7%D0%B0%D0%B4%D0%B0%D1%82%D1%8C%20%D0%B2%D0%B0%D0%BC%20%D0%BD%D0%B5%D0%BA%D0%BE%D1%82%D0%BE%D1%80%D1%8B%D0%B5%20%D0%B2%D0%BE%D0%BF%D1%80%D0%BE%D1%81%D1%8B%20%D0%BE%D0%B1%20URL%20Fixer%2C%20%D1%87%D1%82%D0%BE%D0%B1%D1%8B%20%D0%BB%D1%83%D1%87%D1%88%D0%B5%20%D0%BE%D0%BF%D1%80%D0%B5%D0%B4%D0%B5%D0%BB%D0%B8%D1%82%D1%8C%20%D0%BD%D0%B0%D0%BF%D1%80%D0%B0%D0%B2%D0%BB%D0%B5%D0%BD%D0%B8%D0%B5%20%D0%B4%D0%BB%D1%8F%20%D0%B5%D0%B3%D0%BE%20%D0%B4%D0%B0%D0%BB%D1%8C%D0%BD%D0%B5%D0%B9%D1%88%D0%B5%D0%B3%D0%BE%20%D1%80%D0%B0%D0%B7%D0%B2%D0%B8%D1%82%D0%B8%D1%8F.%20%D0%95%D1%81%D0%BB%D0%B8%20%D1%83%20%D0%B2%D0%B0%D1%81%20%D0%B5%D1%81%D1%82%D1%8C%20%D1%81%D0%B2%D0%BE%D0%B1%D0%BE%D0%B4%D0%BD%D1%8B%D0%B5%205%20%D0%BC%D0%B8%D0%BD%D1%83%D1%82%2C%20%D1%82%D0%BE%20%D0%BD%D0%B0%D0%B6%D0%BC%D0%B8%D1%82%D0%B5%2C%20%D0%BF%D0%BE%D0%B6%D0%B0%D0%BB%D1%83%D0%B9%D1%81%D1%82%D0%B0%2C%20%D0%BD%D0%B0%20%D0%BA%D0%BD%D0%BE%D0%BF%D0%BA%D1%83%2C%20%D1%87%D1%82%D0%BE%D0%B1%D1%8B%20%D0%BF%D1%80%D0%B8%D0%BD%D1%8F%D1%82%D1%8C%20%D1%83%D1%87%D0%B0%D1%81%D1%82%D0%B8%D0%B5%20%D0%B2%20%D0%BE%D0%BF%D1%80%D0%BE%D1%81%D0%B5."),
-    note: decodeURIComponent("%D0%AD%D1%82%D0%BE%20%D0%BE%D0%B4%D0%BD%D0%BE%D1%80%D0%B0%D0%B7%D0%BE%D0%B2%D0%BE%D0%B5%20%D1%81%D0%BE%D0%BE%D0%B1%D1%89%D0%B5%D0%BD%D0%B8%D0%B5%2C%20%D0%BE%D0%BD%D0%BE%20%D0%B1%D0%BE%D0%BB%D1%8C%D1%88%D0%B5%20%D0%BD%D0%B5%20%D0%B1%D1%83%D0%B4%D0%B5%D1%82%20%D0%BF%D0%BE%D0%BA%D0%B0%D0%B7%D1%8B%D0%B2%D0%B0%D1%82%D1%8C%D1%81%D1%8F."),
-    accept: decodeURIComponent("%D0%9F%D1%80%D0%B8%D0%BD%D1%8F%D1%82%D1%8C%20%D1%83%D1%87%D0%B0%D1%81%D1%82%D0%B8%D0%B5%20%D0%B2%20%D0%BE%D0%BF%D1%80%D0%BE%D1%81%D0%B5"),
-    decline: decodeURIComponent("%D0%9C%D0%BE%D0%B6%D0%B5%D1%82%20%D0%B2%20%D0%B4%D1%80%D1%83%D0%B3%D0%BE%D0%B9%20%D1%80%D0%B0%D0%B7")
+exports.incrementCorrectionsCounter = incrementCorrectionsCounter;
+function incrementCorrectionsCounter()
+{
+  // Only users with 5 URL corrections
+  if (++Prefs.corrections_count == 5)
+  {
+    initSurvey();
   }
-};
-
-exports.incrementCorrectionsCounter = incrementCorrectionsCounter;
-function incrementCorrectionsCounter(window)
-{
-  let count = typeof(Prefs.corrections_count)!="undefined" && JSON.parse(Prefs.corrections_count) || 0;
-  
-  // Only users with 5 URL corrections
-  if(++count == 5)
-  {
-    initSurvey(window);
-  }
-
-  Prefs.corrections_count = count;
 }
 
-function initSurvey(window)
+function initSurvey()
 {
-  // Don't do anything on Fennec.
-  if ("Browser" in window || "BrowserApp" in window)
-    return;
-
   // Don't ask after 2012-10-15
   if (Date.now() > 1350259200000)
     return;
 
   // Only Firefox users
-  if (Services.appinfo.ID != "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}")
+  let {application} = require("info");
+  if (application != "firefox")
     return;
 
   // Only Firefox 4 and higher
@@ -74,26 +42,18 @@
   // Delay survey question by 5 seconds
   surveyTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
   surveyTimer.initWithCallback(runSurvey, 5000, Ci.nsITimer.TYPE_ONE_SHOT);
-
-  win = window;
 }
 
 function runSurvey()
 {
-  win.openDialog("chrome://url-fixer/content/survey.xul", "survey", "top=100,left=300,chrome,dialog,dependent");
+  Services.wm.getMostRecentWindow("navigator:browser").openDialog("chrome://url-fixer/content/survey.xul", "survey", "top=100,left=300,chrome,dialog,dependent");

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

a) I'm not insisting on using 80 characters as line length limit but this line
is way too long. Please split it up.
b) getMostRecentWindow() isn't guaranteed to return something, it could also be
null. Please check its result.

Still not fixed from the previous review: the survey question shouldn't be a
dialog.

 }
 
 exports.openSurvey = openSurvey;
 function openSurvey()
 {
-  require("appIntegration").getBrowser(win).loadOneTab(surveyUrl + "&lang=" + surveyLang, {
+  require("appIntegration").getBrowser(Services.wm.getMostRecentWindow("navigator:browser")).loadOneTab(surveyUrl + "&lang=" + surveyLang, {

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

Same here - line too long and result of getMostRecentWindow() unchecked.

       referrerURI: Services.io.newURI("http://url.fixer/", null, null),
       inBackground: false
   });
 }
-
-exports.getSurveyDialogTexts = getSurveyDialogTexts;
-function getSurveyDialogTexts()
-{
-  return langData[surveyLang];
-}