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

Unified Diff: lib/cssInjection.js

Issue 29679796: Issue 6298 - Split injected CSS hiding rule into groups of 1,024 selectors (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Update comment explaining why we split the selectors into groups of 1,024 Created Jan. 30, 2018, 1:39 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 | « include.preload.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/cssInjection.js
===================================================================
--- a/lib/cssInjection.js
+++ b/lib/cssInjection.js
@@ -24,16 +24,45 @@
const {ElemHideEmulation} = require("elemHideEmulation");
const {checkWhitelisted} = require("whitelisting");
const {extractHostFromFrame} = require("url");
const {port} = require("messaging");
const devtools = require("devtools");
const userStyleSheetsSupported = "extensionTypes" in browser &&
"CSSOrigin" in browser.extensionTypes;
+const selectorGroupSize = 1024;
+
+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);
+}
+
+function* createRules(selectors)
+{
+ for (let selectorGroup of splitSelectors(selectors))
+ yield selectorGroup.join(", ") + " {display: none !important;}";
+}
+
+function createStyleSheet(selectors)
+{
+ return Array.from(createRules(selectors)).join("\n");
+}
function addStyleSheet(tabId, frameId, styleSheet)
{
browser.tabs.insertCSS(tabId, {
code: styleSheet,
cssOrigin: "user",
frameId,
matchAboutBlank: true,
@@ -50,17 +79,17 @@
matchAboutBlank: true
});
}
function updateFrameStyles(tabId, frameId, selectors, groupName)
{
let styleSheet = null;
if (selectors.length > 0)
- styleSheet = selectors.join(", ") + "{display: none !important;}";
+ styleSheet = createStyleSheet(selectors);
let frame = ext.getFrame(tabId, frameId);
if (!frame.injectedStyleSheets)
frame.injectedStyleSheets = new Map();
let oldStyleSheet = frame.injectedStyleSheets.get(groupName);
// Ideally we would compare the old and new style sheets and skip this code
« no previous file with comments | « include.preload.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld