Issue 3222 - Mostly unbreak blockable items

chrome/content/ui/sidebar.js

Index: chrome/content/ui/sidebar.js
===================================================================
--- a/chrome/content/ui/sidebar.js
+++ b/chrome/content/ui/sidebar.js
@@ -109,16 +109,24 @@ function init() {
     types.set(type, Utils.getString("type_label_" + type.toLowerCase()));
 }
 
 // To be called for a detached window when the main window has been closed
 function mainUnload() {
   parent.close();
 }
 
+function getFilter(item)
+{
+  if ("filter" in item && item.filter)

[tschuster, 2015/11/12 15:32:05]

"filter" in item is not strictly necessary.

[Wladimir Palant, 2015/11/12 15:46:55]

Yes, I know but I didn't feel like changing that. The code here actually avoids
touching undefined values consistently. Background of this is that Firefox used
to warn for those if javascript.options.strict was enabled.

+    return Filter.fromText(item.filter);
+  else
+    return null;
+}
+
 // To be called on unload
 function cleanUp() {
   flasher.stop();
   requestNotifier.shutdown();
   FilterNotifier.removeListener(reloadDisabledFilters);
   Prefs.removeListener(onPrefChange);
   E("list").view = null;
 
@@ -195,21 +203,27 @@ function onSelectionChange() {
 }
 
 function handleLocationChange()
 {
   if (requestNotifier)
     requestNotifier.shutdown();
 
   treeView.clearData();
+
+  let {getBrowser, addBrowserLocationListener} = require("appSupport");
+  let browser = getBrowser(mainWin);
+  if ("selectedBrowser" in browser)
+    browser = browser.selectedBrowser;
+  let outerWindowID = browser.outerWindowID;
   treeView.itemToSelect = RequestNotifier.getSelection(window.content);
-  requestNotifier = new RequestNotifier(window.content, function(wnd, node, item, scanComplete)
+  requestNotifier = new RequestNotifier(outerWindowID, function(item, scanComplete)
   {
     if (item)
-      treeView.addItem(node, item, scanComplete);
+      treeView.addItem(item, scanComplete);
   });
   cacheStorage = null;
 }
 
 // Fills a box with text splitting it up into multiple lines if necessary
 function setMultilineContent(box, text, noRemove)
 {
   if (!noRemove)
@@ -240,17 +254,17 @@ function fillInTooltip(e) {
     item = treeView.getItemAt(e.clientX, e.clientY);
 
   if (!item)
   {
     e.preventDefault();
     return;
   }
 
-  let filter = ("filter" in item && item.filter ? item.filter : null);
+  let filter = getFilter(item);
   let size = ("tooltip" in item ? null : getItemSize(item));
   let subscriptions = (filter ? filter.subscriptions.filter(function(subscription) { return !subscription.disabled; }) : []);
 
   E("tooltipDummy").hidden = !("tooltip" in item);
   E("tooltipAddressRow").hidden = ("tooltip" in item);
   E("tooltipTypeRow").hidden = ("tooltip" in item);
   E("tooltipSizeRow").hidden = !size;
   E("tooltipDocDomainRow").hidden = ("tooltip" in item || !item.docDomain);
@@ -296,17 +310,17 @@ function fillInTooltip(e) {
         sourceElement.removeChild(sourceElement.firstChild);
       for (let i = 0; i < subscriptions.length; i++)
         setMultilineContent(sourceElement, subscriptions[i].title, true);
     }
   }
 
   var showPreview = Prefs.previewimages && !("tooltip" in item);
   showPreview = showPreview && item.type == "IMAGE";
-  showPreview = showPreview && (!item.filter || item.filter.disabled || item.filter instanceof WhitelistFilter);
+  showPreview = showPreview && (!filter || filter.disabled || filter instanceof WhitelistFilter);
   E("tooltipPreviewBox").hidden = true;
   if (showPreview)
   {
     if (!cacheStorage)
     {
       let {Services} = Cu.import("resource://gre/modules/Services.jsm", null);
       // Cache v2 API is enabled by default starting with Gecko 32
       if (Services.vc.compare(Utils.platformVersion, "32.0a1") >= 0)
@@ -389,19 +403,19 @@ function fillInContext(/**Event*/ e)
   }
 
   if (!item || ("tooltip" in item && !("filter" in item)))
     return false;
 
   E("contextDisableFilter").hidden = true;
   E("contextEnableFilter").hidden = true;
   E("contextDisableOnSite").hidden = true;
-  if ("filter" in item && item.filter)
+  let filter = getFilter(item);
+  if (filter)
   {
-    let filter = item.filter;
     let menuItem = E(filter.disabled ? "contextEnableFilter" : "contextDisableFilter");
     menuItem.setAttribute("label", menuItem.getAttribute("labeltempl").replace(/\?1\?/, filter.text));
     menuItem.hidden = false;
 
     if (filter instanceof ActiveFilter && !filter.disabled && filter.subscriptions.length && !filter.subscriptions.some(subscription => !(subscription instanceof SpecialSubscription)))
     {
       let domain = null;
       try {
@@ -412,38 +426,39 @@ function fillInContext(/**Event*/ e)
       {
         menuItem = E("contextDisableOnSite");
         menuItem.setAttribute("label", menuItem.getAttribute("labeltempl").replace(/\?1\?/, domain));
         menuItem.hidden = false;
       }
     }
   }
 
-  E("contextWhitelist").hidden = ("tooltip" in item || !item.filter || item.filter.disabled || item.filter instanceof WhitelistFilter || item.type == "ELEMHIDE");
+  E("contextWhitelist").hidden = ("tooltip" in item || !filter || filter.disabled || filter instanceof WhitelistFilter || item.type == "ELEMHIDE");
   E("contextBlock").hidden = !E("contextWhitelist").hidden;
-  E("contextBlock").setAttribute("disabled", "filter" in item && item.filter && !item.filter.disabled);
-  E("contextEditFilter").setAttribute("disabled", !("filter" in item && item.filter));
+  E("contextBlock").setAttribute("disabled", filter && !filter.disabled);
+  E("contextEditFilter").setAttribute("disabled", !filter);
   E("contextOpen").setAttribute("disabled", "tooltip" in item || item.type == "ELEMHIDE");
-  E("contextFlash").setAttribute("disabled", "tooltip" in item || !(item.type in visual) || (item.filter && !item.filter.disabled && !(item.filter instanceof WhitelistFilter)));
-  E("contextCopyFilter").setAttribute("disabled", !allItems.some(function(item) {return "filter" in item && item.filter}));
+  E("contextFlash").setAttribute("disabled", "tooltip" in item || !(item.type in visual) || (filter && !filter.disabled && !(filter instanceof WhitelistFilter)));
+  E("contextCopyFilter").setAttribute("disabled", !allItems.some(getFilter));
 
   return true;
 }
 
 /**
  * Processed mouse clicks on the item list.
  * @param {Event} event
  */
 function handleClick(event)
 {
   let item = treeView.getItemAt(event.clientX, event.clientY);
   if (event.button == 0 && treeView.getColumnAt(event.clientX, event.clientY) == "state")
   {
-    if (item.filter)
-      enableFilter(item.filter, item.filter.disabled);
+    let filter = getFilter(item);
+    if (filter)
+      enableFilter(filter, filter.disabled);
     event.preventDefault();
   }
   else if (event.button == 1)
   {
     openInTab(item, event);
     event.preventDefault();
   }
 }
@@ -473,51 +488,49 @@ function openInTab(item, /**Event*/ even
   }
 }
 
 function doBlock() {
   var item = treeView.getSelectedItem();
   if (!item || item.type == "ELEMHIDE")
     return;
 
-  var filter = null;
-  if (item.filter && !item.filter.disabled)
-    filter = item.filter;
-
-  if (filter && filter instanceof WhitelistFilter)
+  var filter = getFilter(item);
+  if (filter && !filter.disabled && filter instanceof WhitelistFilter)
     return;
 
   openDialog("chrome://adblockplus/content/ui/composer.xul", "_blank", "chrome,centerscreen,resizable,dialog=no,dependent", item.nodes, item.orig);
 }
 
 function editFilter()
 {
   var item = treeView.getSelectedItem();
   if (treeView.data && !treeView.data.length)
     item = treeView.getDummyTooltip();
 
-  if (!("filter" in item) || !item.filter)
+  let filter = getFilter(item);
+  if (!filter)
     return;
 
-  UI.openFiltersDialog(item.filter);
+  UI.openFiltersDialog(filter);
 }
 
 function enableFilter(filter, enable) {
   filter.disabled = !enable;
 
   treeView.boxObject.invalidate();
 }
 
 /**
  * Edits the filter to disable it on a particular domain.
  */
 function disableOnSite()
 {
   let item = treeView.getSelectedItem();
-  let filter = item.filter;
+  let filter = getFilter(item);
   if (!(filter instanceof ActiveFilter) || filter.disabled || !filter.subscriptions.length || filter.subscriptions.some(subscription => !(subscription instanceof SpecialSubscription)))
     return;
 
   let domain;
   try {
     domain = Utils.effectiveTLD.getBaseDomainFromHost(item.docDomain).toUpperCase();
   }
   catch (e)
@@ -581,38 +594,38 @@ function disableOnSite()
     let subscription = filter.subscriptions.filter(s => s instanceof SpecialSubscription)[0];
     if (subscription)
       FilterStorage.addFilter(newFilter, subscription, subscription.filters.indexOf(filter));
   }
   FilterStorage.removeFilter(filter);
 
   // Update display
   for (let i = 0; i < treeView.allData.length; i++)
-    if (treeView.allData[i].filter == filter)
+    if (getFilter(treeView.allData[i]) == filter)
       treeView.allData[i].filter = null;
   treeView.boxObject.invalidate();
 }
 
 function copyToClipboard() {
   var items = treeView.getAllSelectedItems();
   if (!items.length)
     return;
 
   Utils.clipboardHelper.copyString(items.map(function(item) {return item.location}).join(IO.lineBreak));
 }
 
 function copyFilter() {
-  var items = treeView.getAllSelectedItems().filter(function(item) {return item.filter});
+  var items = treeView.getAllSelectedItems().filter(getFilter);
   if (treeView.data && !treeView.data.length)
     items = [treeView.getDummyTooltip()];
 
   if (!items.length)
     return;
 
-  Utils.clipboardHelper.copyString(items.map(function(item) {return item.filter.text}).join(IO.lineBreak));
+  Utils.clipboardHelper.copyString(items.map(function(item) {return item.filter}).join(IO.lineBreak));
 }
 
 function selectAll() {
   treeView.selectAll();
 }
 
 // Saves sidebar's state before detaching/reattaching
 function saveState() {
@@ -651,17 +664,18 @@ function detach(doDetach)
   myMainWin.document.getElementById("abp-command-sidebar").doCommand();
   myPrefs.detachsidebar = doDetach;
   myMainWin.document.getElementById("abp-command-sidebar").doCommand();
 }
 
 // Returns items size in the document if available
 function getItemSize(item)
 {
-  if (item.filter && !item.filter.disabled && item.filter instanceof BlockingFilter)
+  let filter = getFilter(item);
+  if (filter && !filter.disabled && filter instanceof BlockingFilter)
     return null;
 
   for (let node of item.nodes)
   {
     if (node instanceof HTMLImageElement && (node.naturalWidth || node.naturalHeight))
       return [node.naturalWidth, node.naturalHeight];
     else if (node instanceof HTMLElement && (node.offsetWidth || node.offsetHeight))
       return [node.offsetWidth, node.offsetHeight];
@@ -694,27 +708,30 @@ function compareType(item1, item2) {
     return 0;
 }
 
 function compareFilter(item1, item2) {
   var hasFilter1 = (item1.filter ? 1 : 0);
   var hasFilter2 = (item2.filter ? 1 : 0);
   if (hasFilter1 != hasFilter2)
     return hasFilter1 - hasFilter2;
-  else if (hasFilter1 && item1.filter.text < item2.filter.text)
+  else if (hasFilter1 && item1.filter < item2.filter)
     return -1;
-  else if (hasFilter1 && item1.filter.text > item2.filter.text)
+  else if (hasFilter1 && item1.filter > item2.filter)
     return 1;
   else
     return 0;
 }
 
-function compareState(item1, item2) {
-  var state1 = (!item1.filter ? 0 : (item1.filter.disabled ? 1 : (item1.filter instanceof WhitelistFilter ? 2 : 3)));
-  var state2 = (!item2.filter ? 0 : (item2.filter.disabled ? 1 : (item2.filter instanceof WhitelistFilter ? 2 : 3)));
+function compareState(item1, item2)
+{
+  let filter1 = getFilter(item1);
+  let filter2 = getFilter(item2);
+  let state1 = (!filter1 ? 0 : (filter1.disabled ? 1 : (filter1 instanceof WhitelistFilter ? 2 : 3)));
+  let state2 = (!filter2 ? 0 : (filter2.disabled ? 1 : (filter2 instanceof WhitelistFilter ? 2 : 3)));
   return state1 - state2;
 }
 
 function compareSize(item1, item2) {
   var size1 = getItemSize(item1);
   size1 = size1 ? size1[0] * size1[1] : 0;
 
   var size2 = getItemSize(item2);
@@ -733,18 +750,20 @@ function compareDocDomain(item1, item2)
   else if (!item1.thirdParty && item2.thirdParty)
     return 1;
   else
     return 0;
 }
 
 function compareFilterSource(item1, item2)
 {
-  let subs1 = item1.filter ? item1.filter.subscriptions.map(s => s.title).join(", ") : "";
-  let subs2 = item2.filter ? item2.filter.subscriptions.map(s => s.title).join(", ") : "";
+  let filter1 = getFilter(item1);
+  let filter2 = getFilter(item2);
+  let subs1 = filter1 ? filter1.subscriptions.map(s => s.title).join(", ") : "";
+  let subs2 = filter2 ? filter2.subscriptions.map(s => s.title).join(", ") : "";
   if (subs1 < subs2)
     return -1;
   else if (subs1 > subs2)
     return 1;
   else
     return 0;
 }
 
@@ -851,30 +870,31 @@ var treeView = {
     if (col != "type" && col != "address" && col != "filter" && col != "size" && col != "docDomain" && col != "filterSource")
       return "";
     if (this.data && this.data.length) {
       if (row >= this.data.length)
         return "";
       if (col == "type")
         return types.get(this.data[row].type);
       else if (col == "filter")
-        return (this.data[row].filter ? this.data[row].filter.text : "");
+        return (this.data[row].filter || "");
       else if (col == "size")
       {
         let size = getItemSize(this.data[row]);
         return (size ? size.join(" x ") : "");
       }
       else if (col == "docDomain")
         return this.data[row].docDomain + " " + (this.data[row].thirdParty ? docDomainThirdParty : docDomainFirstParty);
       else if (col == "filterSource")
       {
-        if (!this.data[row].filter)
+        let filter = getFilter(this.data[row])
+        if (!filter)
           return "";
 
-        return this.data[row].filter.subscriptions.filter(s => !s.disabled).map(s => s.title).join(", ");
+        return filter.subscriptions.filter(s => !s.disabled).map(s => s.title).join(", ");
       }
       else
         return this.data[row].location;
     }
     else {
       // Empty list, show dummy
       if (row > 0 || (col != "address" && col != "filter"))
         return "";
@@ -917,17 +937,17 @@ var treeView = {
 
     let list = [];
     list.push("selected-" + this.selection.isSelected(row));
 
     let state;
     if (this.data && this.data.length) {
       list.push("dummy-false");
 
-      let filter = this.data[row].filter;
+      let filter = getFilter(this.data[row]);
       if (filter)
         list.push("filter-disabled-" + filter.disabled);
 
       state = "state-regular";
       if (filter && !filter.disabled)
       {
         if (filter instanceof WhitelistFilter)
           state = "state-whitelisted";
@@ -1054,41 +1074,44 @@ var treeView = {
     this.allData = [];
     this.dataMap = Object.create(null);
     this.refilter();
 
     this.boxObject.rowCountChanged(0, -oldRows);
     this.boxObject.rowCountChanged(0, this.rowCount);
   },
 
-  addItem: function(/**Node*/ node, /**RequestEntry*/ item, /**Boolean*/ scanComplete)
+  addItem: function(/**RequestEntry*/ item, /**Boolean*/ scanComplete)
   {
     // Merge duplicate entries
     let key = item.location + " " + item.type + " " + item.docDomain;
     if (key in this.dataMap)
     {
       // We know this item already - take over the filter if any and be done with it
       let existing = this.dataMap[key];
       if (item.filter)
         existing.filter = item.filter;
 
-      existing.nodes.push(node);
       this.invalidateItem(existing);
       return;
     }
 
     // Add new item to the list
     // Store original item in orig property - reading out prototype is messed up in Gecko 1.9.2
-    item = {__proto__: item, orig: item, nodes: [node]};
+    item = {__proto__: item, orig: item, nodes: []};

[Wladimir Palant, 2015/11/02 20:01:59]

Not having nodes has two side-effects: the Size column won't show anything, and
flashing an element won't work. These issues have a lower priority, I'll file a
separate bug to address those.

     this.allData.push(item);
     this.dataMap[key] = item;
 
     // Show disabled filters if no other filter applies
     if (!item.filter)
-      item.filter = disabledMatcher.matchesAny(item.location, RegExpFilter.typeMap[item.type], item.docDomain, item.thirdParty);
+    {
+      let disabledMatch = disabledMatcher.matchesAny(item.location, RegExpFilter.typeMap[item.type], item.docDomain, item.thirdParty);
+      if (disabledMatch)
+        item.filter = disabledMatch.text;
+    }
 
     if (!this.matchesFilter(item))
       return;
 
     let index = -1;
     if (this.sortProc && this.sortColumn && this.sortColumn.id == "size")
     {
       // Sorting by size requires accessing content document, and that's
@@ -1130,20 +1153,25 @@ var treeView = {
     else if (!scanComplete && this.selection.currentIndex >= 0) // Keep selected row visible while scanning
       this.boxObject.ensureRowIsVisible(this.selection.currentIndex);
   },
 
   updateFilters: function()
   {
     for (let item of this.allData)
     {
-      if (item.filter instanceof RegExpFilter && item.filter.disabled)
+      let filter = getFilter(item);
+      if (filter instanceof RegExpFilter && filter.disabled)
         delete item.filter;
-      if (!item.filter)
-        item.filter = disabledMatcher.matchesAny(item.location, RegExpFilter.typeMap[item.type], item.docDomain, item.thirdParty);
+      if (!filter)
+      {
+        let disabledMatch = disabledMatcher.matchesAny(item.location, RegExpFilter.typeMap[item.type], item.docDomain, item.thirdParty);
+        if (disabledMatch)
+          item.filter = disabledMatch.text;
+      }
     }
     this.refilter();
   },
 
   /**
    * Updates the list after a filter or sorting change.
    */
   refilter: function()
@@ -1162,17 +1190,17 @@ var treeView = {
    * @return {Boolean} true if the item should be shown
    */
   matchesFilter: function(item)
   {
     if (!this.filter)
       return true;
 
     return (item.location.toLowerCase().indexOf(this.filter) >= 0 ||
-            (item.filter && item.filter.text.toLowerCase().indexOf(this.filter) >= 0) ||
+            (item.filter && item.filter.toLowerCase().indexOf(this.filter) >= 0) ||
             item.type.toLowerCase().indexOf(this.filter.replace(/-/g, "_")) >= 0 ||
             types.get(item.type).toLowerCase().indexOf(this.filter) >= 0 ||
             (item.docDomain && item.docDomain.toLowerCase().indexOf(this.filter) >= 0) ||
             (item.docDomain && item.thirdParty && docDomainThirdParty.toLowerCase().indexOf(this.filter) >= 0) ||
             (item.docDomain && !item.thirdParty && docDomainFirstParty.toLowerCase().indexOf(this.filter) >= 0));
   },
 
   setFilter: function(filter) {
@@ -1241,17 +1269,17 @@ var treeView = {
   },
 
   getDummyTooltip: function() {
     if (!this.data || this.data.length)
       return null;
 
     var filter = Policy.isWindowWhitelisted(window.content);
     if (filter)
-      return {tooltip: this.whitelistDummyTooltip, filter: filter};
+      return {tooltip: this.whitelistDummyTooltip, filter: filter.text};
     else
       return {tooltip: this.itemsDummyTooltip};
   },
 
   invalidateItem: function(item)
   {
     let row = this.data.indexOf(item);
     if (row >= 0)