Added ad counting functionality

stats.js

+/*
+ * This file is part of Adblock Plus <http://adblockplus.org/>,
+ * Copyright (C) 2006-2013 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/>.
+ */
+
+(function()
+{
+  var backgroundPage = chrome.extension.getBackgroundPage();
+  var require = backgroundPage.require;
+  var ChromeCompat = backgroundPage.ChromeCompat;
+  var stats = require("stats");
+  var StatsObject = stats.StatsObject;
+  var Stats = stats.Stats;
+  var FilterNotifier = require("filterNotifier").FilterNotifier;
+  
+  var currentTabId;
+  var statsPage;
+  var statsTotal;
+  var shareBox;
+  var shareURL = "https://adblockplus.org";

[Wladimir Palant, 2013/09/18 13:22:17]

Add a slash at the end of the URL?

+  var redirectURL = "https://www.facebook.com";

[Wladimir Palant, 2013/09/18 13:22:17]

Given that this is Facebook-specific, I don't think that a global variable is
necessary - just add it to the shareLinks variable.

+  
+  function createShareLink(url, params)
+  {
+    var querystring = [];
+    for (var key in params)
+      querystring.push(key + "=" + encodeURIComponent(params[key]));

[Wladimir Palant, 2013/09/18 13:22:17]

encodeURIComponent(key) please, don't assume that it doesn't need to be encoded.

+    return url + "?" + querystring.join("&");
+  }
+  
+  var shareLinks = {
+    facebook: createShareLink("https://www.facebook.com/dialog/feed", {
+      app_id: 475542399197328,
+      link: shareURL,
+      redirect_uri: redirectURL,
+      ref: "adcounter",
+      name: "_MESSAGE_",
+      actions: JSON.stringify([
+        {
+          name: chrome.i18n.getMessage("stats_share_download"),
+          link: shareURL
+        }
+      ])
+    }),
+    gplus: createShareLink("https://plus.google.com/share", {
+      url: shareURL
+    }),
+    twitter: createShareLink("https://twitter.com/intent/tweet", {
+      text: "_MESSAGE_",
+      url: shareURL,
+      via: "AdblockPlus"
+    })
+  };
+  
+  function onLoad()
+  {
+    statsPage = document.querySelector("#statsPage > strong");
+    statsTotal = document.querySelector("#statsTotal > strong");
+    
+    shareBox = document.getElementById("shareBox");
+    var shareBoxLink = document.getElementById("share");
+    shareBoxLink.addEventListener("click", showShareBox, false);
+    
+    var shareActions = document.querySelectorAll("#shareBox > a");
+    for (var i = 0; i < shareActions.length; i++)
+      shareActions[i].addEventListener("click", share, false);

[Wladimir Palant, 2013/09/18 13:22:17]

shareBox.addEventListener("click", share, false) will have exactly the same
effect without hardcoding the page structure - events bubble up.

+    
+    // update stats
+    statsTotal.innerText = formatNumber(Stats.total.blocked);
+    chrome.tabs.query({active: true}, function(tabs)

[Wladimir Palant, 2013/09/18 13:22:17]

You need windowId: chrome.windows.WINDOW_ID_CURRENT as well, otherwise you might
get a tab from a different window. Note that currentWindow property isn't
supported before Chrome 19.

+    {
+      if (tabs.length > 0)
+      {
+        currentTabId = tabs[0].id;
+        updateStats();
+        
+        FilterNotifier.addListener(onNotify);
+        
+        document.getElementById("statsStuff").removeAttribute("hidden");
+      }
+    });
+  }
+  
+  function onUnload()
+  {
+    FilterNotifier.removeListener(onNotify);
+  }
+  
+  function onNotify(action, item)
+  {
+    if (action == "document.stats")
+      updateStats();
+  }
+  
+  function updateStats()
+  {
+    ChromeCompat.tabs.sendMessage(currentTabId, {reqtype: "get-stats"}, function(response)
+    {
+      var page = new StatsObject(response);
+      statsPage.innerText = formatNumber(page.blocked);
+      statsTotal.innerText = formatNumber(Stats.total.blocked);

[Wladimir Palant, 2013/09/18 13:22:17]

So that's why we have placeholders, so that we can later go into the text and
assume that the placeholder was inside the <strong> block :)

I guess that the best approach would be extending public functionality in
i18n.js, e.g. but adding a public setElementText() function:

function setElementText(element, stringName, arguments)
{
  function processString(...)
  {
    ...
  }

  while (element.lastChild)
    element.removeChild(element.lastChild);
  processString(i18n.getMessage(stringName, arguments), element);
}

loadI18nStrings() could then call this function - and you could as well,
whenever you need it.

+    });
+  }
+  
+  function showShareBox(ev)
+  {
+    if (shareBox.hasAttribute("hidden"))
+      shareBox.removeAttribute("hidden");
+    else
+      shareBox.setAttribute("hidden", true);

[Wladimir Palant, 2013/09/18 13:22:17]

How about:

var shareBox = document.getElementById("shareBox");
shareBox.hidden = !shareBox.hidden;

The name should probably be toggleShareBox() to reflect the functionality. Note
that I'm opposed to caching all elements in global variables, it's typically
unnecessary (doesn't save any code and is irrelevant performance-wise) and makes
the code harder to read.

+  }
+  
+  function share(ev)
+  {
+    // Easter Egg
+    var blocked = Stats.total.blocked;
+    if (blocked <= 9000 || blocked >= 10000)
+      blocked = formatNumber(blocked);
+    else
+      blocked = chrome.i18n.getMessage("stats_over", formatNumber(9000));
+    
+    var url = shareLinks[ev.target.dataset.social]
+      .replace("_MESSAGE_", chrome.i18n.getMessage("stats_share_message", blocked));

[Wladimir Palant, 2013/09/18 13:22:17]

This is a rather ugly approach, and you forgot to URL-encode the message here.
How about generating the links here rather then up front?. Then
createShareLink() could insert the message during link generation:

var messageMark = {};
var shareLinks = {
  facebook: ["http://...", {
    ...
    name: messageMark,
    ...
  }]
};

function createShareLink(network, blockedCount)
{
  var url = shareLinks[network][0];
  var params = shareLinks[network][1];
  ...
  var value = params[key];
  if (value == messageMark)
    value = i18n.getMessage("stats_share_message", blockedCount);
  ...
}

+    
+    chrome.tabs.create({url: url});
+  }
+  
+  var locale = require("utils").Utils.appLocale;
+  function formatNumber(number)
+  {
+    return number.toLocaleString(locale);
+  }

[Wladimir Palant, 2013/09/18 13:22:17]

Utils.appLocale returns the browser's locale. So this code would only make sense
if toLocaleString() uses the system locale by default and not the browser
locale. In my tests this doesn't seem to be the case. In other words,
formatNumber(number) does exactly the same as number.toLocaleString().

+  
+  document.addEventListener("DOMContentLoaded", onLoad, false);
+  window.addEventListener("unload", onUnload, false);
+})();