Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/abp2blocklist.js

Issue 29473555: Issue 5345 - Whitelist $elemhide and $generichide domains where possible (Closed) Base URL: https://hg.adblockplus.org/abp2blocklist
Patch Set: Generate unless-domain value outside while loop Created July 11, 2017, 11:14 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « abp2blocklist.js ('k') | test/abp2blocklist.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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,41 @@
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)
{
+ let unlessDomain = exceptionDomains.size > 0 ? [] : null;
kzar 2017/07/11 12:20:03 Nit: This seems like overkill since `[]` is falsey
Manish Jethani 2017/07/11 16:28:39 [] evaluates to true. If you mean checking the len
kzar 2017/07/11 16:30:36 Whoops, you're right.
+
+ exceptionDomains.forEach(name => unlessDomain.push("*" + name));
+
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 (unlessDomain)
+ rule.trigger["unless-domain"] = unlessDomain;
+
+ rules.push(rule);
}
}
let ContentBlockerList =
/**
* Create a new Adblock Plus filter to content blocker list converter
*
* @constructor
@@ -507,31 +528,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);
+ });
+
+ 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);
}
« no previous file with comments | « abp2blocklist.js ('k') | test/abp2blocklist.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld