Issue 3585 - Merge element hiding rules for the same domain

abp2blocklist.js

+"use strict";
+
 var readline = require("readline");
 var punycode = require("punycode");
 var tldjs = require("tldjs");
 var filterClasses = require("./adblockplus.js");
 
 var typeMap = filterClasses.RegExpFilter.typeMap;
 
 var requestFilters = [];
 var requestExceptions = [];
 var elemhideFilters = [];
@@ skipping 80 matching lines @@
 
 function convertElemHideFilter(filter)
 {
   var included = [];
   var excluded = [];
   var rules = [];
 
   parseDomains(filter.domains, included, excluded);
 
   if (excluded.length == 0 && !(filter.selector in elemhideSelectorExceptions))
-  {
-    var action = {
-      type: "css-display-none",
-      selector: filter.selector
-    };
-
-    for (var i = 0; i < included.length; i++)
-      rules.push({
-          trigger: {"url-filter": matchDomain(included[i])},
-          action: action
-      });
-
-    if (included.length == 0)
-      rules.push({
-          trigger: {"url-filter": "^https?://"},
-          action: action
-      });
-  }
-
-  return rules;
+    return [included.map(matchDomain), filter.selector];
 }
 
 function toRegExp(text)
 {
   var result = "";
   var lastIndex = text.length - 1;
 
   for (var i = 0; i < text.length; i++)
   {
     var c = text[i];
@@ skipping 145 matching lines @@
 {
   var rules = [];
   var i;
 
   function addRule(rule)
   {
     if (!hasNonASCI(rule))
       rules.push(rule);
   }
 
-  // HACK: We ignore element hiding filter for now to get the list of
-  // rules down below 50K. This limit is enforced by iOS and Safari.
-  // To be undone with https://issues.adblockplus.org/ticket/3585
+  let groupedElemhideFilters = new Map();
+  for (let filter of elemhideFilters)

[Sebastian Noack, 2016/02/15 14:33:11]

Does these ES2015 features even work in node.js without any additional command
line options? I remember that when I initially wrote that code I couldn't use
let statements.

Moreover, regardless of ES2015 support in the latest node version, I think we
should either stick to var, or use let consistently in the whole file.

[kzar, 2016/02/15 18:19:20]

(As discussed in IRC we will use ES2015 features that are supported by node.js
stable without special arguments.)

+  {
+    let result = convertElemHideFilter(filter);
+    if (!result)
+      continue;
+    let targetDomains = result[0];
+    let selector = result[1];
 
-  //for (i = 0; i < elemhideFilters.length; i++)
-  //  convertElemHideFilter(elemhideFilters[i]).forEach(addRule);
-  //for (i = 0; i < elemhideExceptions.length; i++)
-  //  addRule(convertFilter(elemhideExceptions[i], "ignore-previous-rules", false));
+    if (targetDomains.length == 0)
+      targetDomains = ["^https?://"];
+
+    for (let domain of targetDomains)
+    {
+      if (!groupedElemhideFilters.has(domain))
+        groupedElemhideFilters.set(domain, []);
+      groupedElemhideFilters.get(domain).push(selector);
+    }
+  }
+
+  groupedElemhideFilters.forEach((selectors, domain) =>
+  {
+    let rule = {trigger: {"url-filter": domain},
+                action: {type: "css-display-none"}};
+
+    if (selectors.length == 1)
+      rule["action"]["selector"] = selectors[0];
+    else
+      rule["action"]["selector"] = ":matches(" + selectors.join(", ") + ")";

[kzar, 2016/02/13 19:33:24]

Note: I wasn't sure if we need to escape the selectors here.

[Sebastian Noack, 2016/02/15 14:33:11]

Well, if any CSS selector is invalid it will break the whole group of selectors.
But that is the same with the logic we have on Chrome. So I don't think we need
to bother. And as far as I understand :matches, it's not possible to prevent
that by escaping as we need raw selectors here.

However, I wonder whether we even need :matches() or whether we can simply join
the selectors by comma. It seems that is what AdBlock is doing. While looking at
their code I also figured that they never merge more than 1000 selectors into
one rule. Perhaps, also Dean can give some advice here?

[kzar, 2016/02/15 18:19:20]

Acknowledged.

+
+    addRule(rule);
+  });
+
+  for (i = 0; i < elemhideExceptions.length; i++)
+    addRule(convertFilter(elemhideExceptions[i], "ignore-previous-rules", false));
 
   for (i = 0; i < requestFilters.length; i++)
     addRule(convertFilter(requestFilters[i], "block", true));
   for (i = 0; i < requestExceptions.length; i++)
     addRule(convertFilter(requestExceptions[i], "ignore-previous-rules", true));
 
   console.log(JSON.stringify(rules, null, "\t"));
 }
 
 var rl = readline.createInterface({input: process.stdin, terminal: false});
 rl.on("line", parseFilter);
 rl.on("close", logRules);