Issue 3269 - Display uninstallation page when uninstalled

lib/uninstall.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /** @module uninstall */
 
+let info = require("info");
 let {Prefs} = require("prefs");
+let {Utils} = require("utils");
 
 function setUninstallURL()
 {
-  let info = require("info");
   let search = [];
-  for (let key of ["addonName", "addonVersion", "application",

[Sebastian Noack, 2015/12/12 13:43:27]

I just rembered that jshydra generates quite crappy code if you use array
literals in for-of loops:

  for (var _loopIndex55 = 0; _loopIndex55 < ["addonName", "addonVersion",
"application", "applicationVersion", "platform", "platformVersion"].length;
++_loopIndex55)
  { 
    var key = ["addonName", "addonVersion", "application", "applicationVersion",
"platform", "platformVersion"][_loopIndex55];
    search.push(key + "=" + encodeURIComponent(info[key]));
  }

It basically duplicates the code in the of clause. So I suggest to store that
array in a variable. Or alternatively, just omit the loop like we do in
adblockplus/lib/downloader.js.

[kzar, 2015/12/12 14:13:43]

Done.

-                   "applicationVersion", "platform", "platformVersion"])
+  let keys = ["addonName", "addonVersion", "application", "applicationVersion",
+              "platform", "platformVersion"];
+  for (let key of keys)
     search.push(key + "=" + encodeURIComponent(info[key]));
 
   let downlCount = Prefs.notificationdata.downloadCount || 0;
   search.push("notificationDownloadCount=" + encodeURIComponent(downlCount));
 
-  chrome.runtime.setUninstallURL(
-    require("utils").Utils.getDocLink("uninstall-abp") + "&" + search.join("&")

[Sebastian Noack, 2015/12/12 13:43:27]

As mentioned in the issue, I think we should just call it "uninstalled".

[Sebastian Noack, 2015/12/12 13:43:27]

I'd rather keep imports at the top, even if only used once, if it's only for
consistency. But that way it'd be also more obvious which module depends on
which other module.

[kzar, 2015/12/12 14:13:43]

Done.

[kzar, 2015/12/12 14:13:43]

Done. (Provisionally, if they disagree on the issue we might need to change it
back.)

-  );
+  chrome.runtime.setUninstallURL(Utils.getDocLink("uninstalled") + "&" +
+                                 search.join("&"));
 }
 
 function onPrefsLoaded()
 {
   Prefs.onLoaded.removeListener(onPrefsLoaded);
   setUninstallURL();
 }
 
 // The uninstall URL contains the notification download count as a parameter,
 // therefore we must wait for preferences to be loaded before generating the
 // URL and we need to re-generate it each time the notification data changes.
-Prefs.onLoaded.addListener(onPrefsLoaded);
-Prefs.onChanged.addListener(function(name)
+if ("setUninstallURL" in chrome.runtime)
 {
-  if (name == "notificationdata")
-    setUninstallURL();
-});
+  Prefs.onLoaded.addListener(onPrefsLoaded);
+  Prefs.onChanged.addListener(function(name)
+  {
+    if (name == "notificationdata")
+      setUninstallURL();
+  });
+}