Applied changes from emailed code review

lib/survey.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");
 
 let {Prefs} = require("prefs");
 
 let surveyLang = null;
-let surveyUrl = "http://urlfixer.org"; //"http://adblockplus.org/usersurvey/index.php?sid=68316";
+let surveyUrl = null; //"http://urlfixer.org/usersurvey/...";
 
 exports.incrementCorrectionsCounter = incrementCorrectionsCounter;
 function incrementCorrectionsCounter()
 {
+  // Only if survey exists
+  if (!surveyUrl)
+    return;
+  
   // Only users with 5 URL corrections
   if (++Prefs.corrections_count == 5)
   {
     initSurvey();
   }
 }
 
 function initSurvey()
 {
   // Don't ask after 2012-10-15
@@ skipping 14 matching lines @@
     return;
   surveyLang = RegExp.$1;
 
   // Delay survey question by 5 seconds
   surveyTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
   surveyTimer.initWithCallback(runSurvey, 5000, Ci.nsITimer.TYPE_ONE_SHOT);
 }
 
 function runSurvey()
 {
-  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.

+  //open panel
+  let request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance(Ci.nsIXMLHttpRequest);
+  request.open("GET", "chrome://url-fixer/content/survey.xul");
+  request.addEventListener("load", function(event)
+  {
+    let window = Services.wm.getMostRecentWindow("navigator:browser");
+    let document = window.document;
+
+    let style = document.createProcessingInstruction('xml-stylesheet', 'href="chrome://url-fixer/skin/survey.css" type="text/css"');
+    document.insertBefore(style, document.firstChild);
+
+    let panel = new window.DOMParser().parseFromString(request.responseText, "text/xml").documentElement;
+    let oldPanel = document.getElementById(panel.id);
+    if (oldPanel)
+      oldPanel.parentNode.removeChild(oldPanel);
+    document.getElementById("mainPopupSet").appendChild(panel);
+    document.getElementById("url-fixer-icon").setAttribute("src", require("info").addonRoot + "icon64.png");
+    document.getElementById("url-fixer-accept-button").addEventListener("command", function()
+    {
+      openSurvey();
+      panel.hidePopup();
+    });
+    document.getElementById("url-fixer-cancel-button").addEventListener("command", function()
+    {
+      panel.hidePopup();
+    });
+    panel.addEventListener("popuphidden", function()
+    {
+      panel.parentNode.removeChild(panel);
+      style.parentNode.removeChild(style);
+    });
+
+    let anchor = document.getElementById("identity-box");
+    panel.openPopup(anchor, "after_start", 0, 0, false, true);
+  }, false);
+  request.send(null);
 }
 
-exports.openSurvey = openSurvey;
 function openSurvey()
 {
-  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
-  });
+  let window = Services.wm.getMostRecentWindow("navigator:browser");
+  if (window)
+  {
+    let browser = require("appIntegration").getBrowser(window);
+    browser.loadOneTab(surveyUrl + "&lang=" + surveyLang, {
+        referrerURI: Services.io.newURI("http://url.fixer/", null, null),
+        inBackground: false
+    });
+  }
 }