Issue 5345 - Whitelist $elemhide and $generichide domains where possible

lib/abp2blocklist.js

Index: lib/abp2blocklist.js
===================================================================
--- a/lib/abp2blocklist.js
+++ b/lib/abp2blocklist.js
@@ -73,16 +73,28 @@
   {
     if (name.length > suffixLength && name.slice(-suffixLength) == "." + domain)
       subdomains.push(name.slice(0, -suffixLength));
   }
 
   return subdomains;
 }
 
+function extractFilterDomains(filters)
+{
+  let domains = new Set();
+  for (let filter of filters)
+  {
+    let parsed = parseFilterRegexpSource(filter.regexpSource);
+    if (parsed.justHostname)
+      domains.add(parsed.hostname);
+  }
+  return domains;
+}
+
 function convertElemHideFilter(filter, elemhideSelectorExceptions)
 {
   let included = [];
   let excluded = [];
   let rules = [];
 
   parseDomains(filter.domains, included, excluded);
 
@@ -128,17 +140,17 @@
     if (hostnameStart != null && !hostnameFinished)
     {
       let endingChar = (c == "*" || c == "^" ||
                         c == "?" || c == "/" || c == "|");
       if (!endingChar && i != lastIndex)
         continue;
 
       hostname = punycode.toASCII(
-        text.substring(hostnameStart, endingChar ? i : i + 1)
+        text.substring(hostnameStart, endingChar ? i : i + 1).toLowerCase()
       );
       hostnameFinished = justHostname = true;
       regexp.push(escapeRegExp(hostname));
       if (!endingChar)
         break;
     }
 
     switch (c)
@@ -395,32 +407,43 @@
     newSelector.push('[id=', selector.substring(pos.start + 1, pos.end), ']');
     i = pos.end;
   }
   newSelector.push(selector.substring(i));
 
   return newSelector.join("");
 }
 
-function addCSSRules(rules, selectors, matchDomain)
+function addCSSRules(rules, selectors, matchDomain, exceptionDomains)
 {
   while (selectors.length)
   {
     let selector = selectors.splice(0, selectorLimit).join(", ");
 
     // As of Safari 9.0 element IDs are matched as lowercase. We work around
     // this by converting to the attribute format [id="elementID"]
     selector = convertIDSelectorsToAttributeSelectors(selector);
 
-    rules.push({
+    let rule = {
       trigger: {"url-filter": matchDomain,
                 "url-filter-is-case-sensitive": true},
       action: {type: "css-display-none",
                selector: selector}
-    });
+    };
+
+    if (exceptionDomains.size > 0)
+    {
+      rule.trigger["unless-domain"] = [];
+      exceptionDomains.forEach(name =>
+      {
+        rule.trigger["unless-domain"].push("*" + name);
+      });
+    }
+
+    rules.push(rule);
   }
 }
 
 let ContentBlockerList =
 /**
  * Create a new Adblock Plus filter to content blocker list converter
  *
  * @constructor
@@ -507,31 +530,45 @@
       {
         let group = groupedElemhideFilters.get(matchDomain) || [];
         group.push(result.selector);
         groupedElemhideFilters.set(matchDomain, group);
       }
     }
   }
 
-  addCSSRules(rules, genericSelectors, "^https?://");
+  // Separate out the element hiding exceptions that have only a hostname part
+  // from the rest. This allows us to implement a workaround for issue #5345
+  // (WebKit bug #167423), but as a bonus it also reduces the number of
+  // generated rules. The downside is that the exception will only apply to the
+  // top-level document, not to iframes. We have to live with this until the
+  // WebKit bug is fixed in all supported versions of Safari.
+  // https://bugs.webkit.org/show_bug.cgi?id=167423
+  //
+  // Note that as a result of this workaround we end up with a huge rule set in
+  // terms of the amount of memory used. This can cause Node.js to throw
+  // "JavaScript heap out of memory". To avoid this, call Node.js with
+  // --max_old_space_size=4096
+  let elemhideExceptionDomains = extractFilterDomains(this.elemhideExceptions);
 
-  // Right after the generic element hiding filters, add the exceptions that
-  // should apply only to those filters.
-  for (let filter of this.generichideExceptions)
-    convertFilterAddRules(rules, filter, "ignore-previous-rules", false);
+  let genericSelectorExceptionDomains =
+    extractFilterDomains(this.generichideExceptions);
+  elemhideExceptionDomains.forEach(name =>
+  {
+    genericSelectorExceptionDomains.add(name);

[kzar, 2017/07/10 12:33:08]

I wonder if it would be better to pass two Sets of domains to addCSSRules rather
than combining them here. I figure we're iterating through those domains twice
otherwise.

[Manish Jethani, 2017/07/11 11:19:18]

We'd still have to combine them into one set if we want to filter out
duplicates. Also, this code runs only once per rule set generation, it really
doesn't matter.

Alternatively we could use the has method to check if the value is already in
the other set while iterating.

[kzar, 2017/07/11 12:20:03]

Fair enough.

+  });
+
+  addCSSRules(rules, genericSelectors, "^https?://",
+              genericSelectorExceptionDomains);
 
   groupedElemhideFilters.forEach((selectors, matchDomain) =>
   {
-    addCSSRules(rules, selectors, matchDomain);
+    addCSSRules(rules, selectors, matchDomain, elemhideExceptionDomains);
   });
 
-  for (let filter of this.elemhideExceptions)
-    convertFilterAddRules(rules, filter, "ignore-previous-rules", false);
-
   let requestFilterExceptionDomains = [];
   for (let filter of this.genericblockExceptions)
   {
     let parsed = parseFilterRegexpSource(filter.regexpSource);
     if (parsed.hostname)
       requestFilterExceptionDomains.push(parsed.hostname);
   }