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

Unified Diff: lib/elemHide.js

Issue 29882562: Issue 6956 - Move extension's style sheet generation into core (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Created Sept. 16, 2018, 5:19 p.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 | « no previous file | test/elemHide.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/elemHide.js
===================================================================
--- a/lib/elemHide.js
+++ b/lib/elemHide.js
@@ -20,16 +20,25 @@
/**
* @fileOverview Element hiding implementation.
*/
const {ElemHideExceptions} = require("./elemHideExceptions");
const {filterNotifier} = require("./filterNotifier");
/**
+ * The maximum number of selectors in a CSS rule. This is used by
+ * <code>{@link ElemHide.createStyleSheet}</code> to split up a long list of
+ * selectors into multiple rules.
+ * @const {number}
+ * @default
+ */
+const selectorGroupSize = 1024;
+
+/**
* Lookup table, active flag, by filter by domain.
* (Only contains filters that aren't unconditionally matched for all domains.)
* @type {Map.<string,Map.<Filter,boolean>>}
*/
let filtersByDomain = new Map();
/**
* Lookup table, filter by selector. (Only used for selectors that are
@@ -55,16 +64,54 @@
/**
* Set containing known element hiding filters
* @type {Set.<ElemHideFilter>}
*/
let knownFilters = new Set();
/**
+ * Splits a list of selectors into groups determined by the value of
+ * <code>{@link selectorGroupSize}</code>.
+ *
+ * @param {Array.<string>} selectors
+ * @yields {Array.<string>}
+ */
+function* splitSelectors(selectors)
+{
+ // Chromium's Blink engine supports only up to 8,192 simple selectors, and
+ // even fewer compound selectors, in a rule. The exact number of selectors
+ // that would work depends on their sizes (e.g. "#foo .bar" has a size of 2).
+ // Since we don't know the sizes of the selectors here, we simply split them
+ // into groups of 1,024, based on the reasonable assumption that the average
+ // selector won't have a size greater than 8. The alternative would be to
+ // calculate the sizes of the selectors and divide them up accordingly, but
+ // this approach is more efficient and has worked well in practice. In theory
+ // this could still lead to some selectors not working on Chromium, but it is
+ // highly unlikely.
+ // See issue #6298 and https://crbug.com/804179
+ for (let i = 0; i < selectors.length; i += selectorGroupSize)
+ yield selectors.slice(i, i + selectorGroupSize);
Sebastian Noack 2018/09/17 18:59:57 Apparently this code is currently already using ge
+}
+
+/**
+ * Creates element hiding CSS rules for a given list of selectors. Each rule
+ * contains no more than the maximum number of selectors as determined by the
+ * value of <code>{@link selectorGroupSize}</code>.
+ *
+ * @param {Array.<string>} selectors
+ * @yields {string}
+ */
+function* createRules(selectors)
+{
+ for (let selectorGroup of splitSelectors(selectors))
+ yield selectorGroup.join(", ") + " {display: none !important;}";
+}
+
+/**
* Adds a filter to the lookup table of filters by domain.
* @param {Filter} filter
*/
function addToFiltersByDomain(filter)
{
let domains = filter.domains || defaultDomains;
for (let [domain, isIncluded] of domains)
{
@@ -234,10 +281,20 @@
let nextDot = currentDomain.indexOf(".");
currentDomain = nextDot == -1 ? "" : currentDomain.substr(nextDot + 1);
}
if (!specificOnly)
selectors = getUnconditionalSelectors().concat(selectors);
return selectors;
+ },
+
+ /**
+ * Creates an element hiding CSS style sheet from a given list of selectors.
+ * @param {Array.<string>} selectors
+ * @returns {string}
+ */
+ createStyleSheet(selectors)
+ {
+ return [...createRules(selectors)].join("\n");
}
};
« no previous file with comments | « no previous file | test/elemHide.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld