include.preload.js - Issue 5504296699297792: Issue 1802 - Fix element hiding filters with multiple CSS selectors when using shadow DOM

Side by Side Diff: include.preload.js

Issue 5504296699297792: Issue 1802 - Fix element hiding filters with multiple CSS selectors when using shadow DOM (Closed)

Patch Set: Added comment explaining regex Created Jan. 15, 2015, 12:56 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
1 /*	1 /*

2 * This file is part of Adblock Plus <http://adblockplus.org/>,	2 * This file is part of Adblock Plus <http://adblockplus.org/>,

3 * Copyright (C) 2006-2014 Eyeo GmbH	3 * Copyright (C) 2006-2014 Eyeo GmbH

4 *	4 *

5 * Adblock Plus is free software: you can redistribute it and/or modify	5 * Adblock Plus is free software: you can redistribute it and/or modify

6 * it under the terms of the GNU General Public License version 3 as	6 * it under the terms of the GNU General Public License version 3 as

7 * published by the Free Software Foundation.	7 * published by the Free Software Foundation.

8 *	8 *

9 * Adblock Plus is distributed in the hope that it will be useful,	9 * Adblock Plus is distributed in the hope that it will be useful,

10 * but WITHOUT ANY WARRANTY; without even the implied warranty of	10 * but WITHOUT ANY WARRANTY; without even the implied warranty of

(...skipping 164 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
175 if (selectors.length == 0)	175 if (selectors.length == 0)

176 return;	176 return;

177	177

178 var style = document.createElement("style");	178 var style = document.createElement("style");

179 style.setAttribute("type", "text/css");	179 style.setAttribute("type", "text/css");

180	180

181 if (shadow)	181 if (shadow)

182 {	182 {

183 shadow.appendChild(style);	183 shadow.appendChild(style);

184	184

	185 // Add ::content pseudo-selector (to break out of the shadow DOM)

	186 // before every selector, also considering element hiding filters

	187 // that specify multiple CSS selectors seperated by comma, while

	188 // ignoring commas inside quotes.

185 for (var i = 0; i < selectors.length; i++)	189 for (var i = 0; i < selectors.length; i++)

186 selectors[i] = "::content " + selectors[i];	190 selectors[i] = selectors[i].replace(/(\s)((?:[^,"']\|"(?:\\"\|[^"])"\|'(? :\\'\|[^'])*')+)/g, "$1::content $2");
	Wladimir Palant 2015/01/19 20:49:17 Trying to parse CSS selectors via regular expressi Trying to parse CSS selectors via regular expressions seems to be a lost cause to me, in the best scenario we will merely end up with code that is unmaintainable. In addition to being barely readable, this particular regular expression has at least the following issues: * It treats backslashes in strings incorrectly (e.g. foo['as\\'], bar['']). * It doesn't consider escaping outside of strings (e.g. "foo[bar\\,bas]"). People have written huge CSS selector parsers, CSS really isn't simple. Granted, for our use case this complexity might not be required. In fact, we don't even need to recognize CSS escaping properly - it's enough to consider the next character to be escaped. I have a strong suspicion that a state machine would be more readable and maintainable here. We wouldn't need too many states: looks like SELECTOR and STRING would be sufficient (the latter either with ' or " as end character). A backslash should simply make the state machine skip the next character. Note that http://dev.w3.org/csswg/css-syntax-3/ specifies a state machine. The hard part is figuring out which parts of it we need. Wladimir Palant 2015/01/19 20:58:15 Just my quick attempt at writing that state machin Just my quick attempt at writing that state machine: function getSelectors(str) { var result = []; var start = 0; var separator = ""; for (var i = 0; i < str.length; i++) { var char = str[i]; if (char == "\\") i++; // Escaped char else if (char == separator) separator = ""; // String ended else if (char == "'" \|\| char == '"') separator = char; // String started else if (separator == "" && char == ",") { // Selector ended result.push(str.substring(start, i)); start = i + 1; } } if (start < str.length) result.push(str.substring(start, str.length)); return result; } Sebastian Noack 2015/01/22 07:37:12 Unfortunately it seems we in fact need a state mac Show quoted text On 2015/01/19 20:58:15, Wladimir Palant wrote: > Just my quick attempt at writing that state machine: > > function getSelectors(str) > { > var result = []; > var start = 0; > var separator = ""; > for (var i = 0; i < str.length; i++) > { > var char = str[i]; > if (char == "\\") > i++; // Escaped char > else if (char == separator) > separator = ""; // String ended > else if (char == "'" \|\| char == '"') > separator = char; // String started > else if (separator == "" && char == ",") > { // Selector ended > result.push(str.substring(start, i)); > start = i + 1; > } > } > if (start < str.length) > result.push(str.substring(start, str.length)); > return result; > } Unfortunately it seems we in fact need a state machine here. There you go.
187 }	191 }

188 else	192 else

189 {	193 {

190 // Try to insert the style into the <head> tag, inserting directly under t he	194 // Try to insert the style into the <head> tag, inserting directly under t he

191 // document root breaks dev tools functionality:	195 // document root breaks dev tools functionality:

192 // http://code.google.com/p/chromium/issues/detail?id=178109	196 // http://code.google.com/p/chromium/issues/detail?id=178109

193 (document.head \|\| document.documentElement).appendChild(style);	197 (document.head \|\| document.documentElement).appendChild(style);

194 }	198 }

195	199

196 var setRules = function()	200 var setRules = function()

(...skipping 45 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
242 }, true);	246 }, true);

243	247

244 ext.backgroundPage.sendMessage({type: "get-selectors"}, setElemhideCSSRules);	248 ext.backgroundPage.sendMessage({type: "get-selectors"}, setElemhideCSSRules);

245 }	249 }

246	250

247 if (document instanceof HTMLDocument)	251 if (document instanceof HTMLDocument)

248 {	252 {

249 checkSitekey();	253 checkSitekey();

250 init(document);	254 init(document);

251 }	255 }

OLD	NEW

« no previous file with comments | « no previous file | no next file » | no next file with comments »