include.preload.js - Issue 5989914281771008: Use Shadow DOM for element hiding

Delta Between Two Patch Sets: include.preload.js

Issue 5989914281771008: Use Shadow DOM for element hiding (Closed)

Left Patch Set: Created April 15, 2014, 9:48 a.m.

Right Patch Set: Fixed Shadow DOM usage and simplified code Created April 15, 2014, 7:06 p.m.

Left:
Right:

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

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
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

11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the	11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

12 * GNU General Public License for more details.	12 * GNU General Public License for more details.

13 *	13 *

14 * You should have received a copy of the GNU General Public License	14 * You should have received a copy of the GNU General Public License

15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.	15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.

16 */	16 */

17	17

18 var SELECTOR_GROUP_SIZE = 20;	18 var SELECTOR_GROUP_SIZE = 20;

19	19

20 var elemhideElt = null;

21

22 // Sets the currently used CSS rules for elemhide filters	20 // Sets the currently used CSS rules for elemhide filters

23 function setElemhideCSSRules(selectors)	21 function setElemhideCSSRules(selectors)

24 {	22 {

25 if (elemhideElt && elemhideElt.parentNode)

26 elemhideElt.parentNode.removeChild(elemhideElt);

27

28 if (selectors.length == 0)	23 if (selectors.length == 0)
Sebastian Noack 2014/04/15 09:58:06 Skip inserting a style tag, if there are no elemen Skip inserting a style tag, if there are no element hiding rules for the current website. Previously we only bailed out when the website itself is whitelisted. Wladimir Palant 2014/04/15 13:13:14 That doesn't make much difference in reality - if Show quoted text On 2014/04/15 09:58:06, Sebastian Noack wrote: > Skip inserting a style tag, if there are no element hiding rules for the current > website. Previously we only bailed out when the website itself is whitelisted. That doesn't make much difference in reality - if the user has a filter list and didn't whitelist the website then some element hiding rules will most certainly apply to every domain. Still nothing wrong with the change of course. Sebastian Noack 2014/04/15 19:21:54 I am aware that at least when using EASYList there Show quoted text On 2014/04/15 13:13:14, Wladimir Palant wrote: > On 2014/04/15 09:58:06, Sebastian Noack wrote: > > Skip inserting a style tag, if there are no element hiding rules for the > current > > website. Previously we only bailed out when the website itself is whitelisted. > > That doesn't make much difference in reality - if the user has a filter list and > didn't whitelist the website then some element hiding rules will most certainly > apply to every domain. Still nothing wrong with the change of course. I am aware that at least when using EASYList there are always some element hiding filters that are included on all websites. However handling that case doesn't add any complexity, and IMHO it makes the API even more simple and robust against wrong assumptions when always an array is returned.
29 return;	24 return;

30	25

31 // Try to insert the style into the <head> tag, inserting directly under the	26 var style = document.createElement("style");

32 // document root breaks dev tools functionality:	27 style.setAttribute("type", "text/css");

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

34 var container = document.head \|\| document.documentElement;

35	28

36 // Use Shadow DOM if available to don't mess with web pages	29 // Use Shadow DOM if available to don't mess with web pages

37 // that rely on the order of their own <style> tags (#309)	30 // that rely on the order of their own <style> tags (#309)

38 if ("createShadowRoot" in container)	31 if ("webkitCreateShadowRoot" in document.documentElement)

39 container = container.createShadowRoot();	32 {
Wladimir Palant 2014/04/15 13:13:14 Given that this standard is still very much in flu Given that this standard is still very much in flux we should not expect our code to work with the final version of Web Components. It should be better to check for webkitCreateShadowRoot only, at least for now. Sebastian Noack 2014/04/15 19:21:54 Done. Show quoted text On 2014/04/15 13:13:14, Wladimir Palant wrote: > Given that this standard is still very much in flux we should not expect our > code to work with the final version of Web Components. It should be better to > check for webkitCreateShadowRoot only, at least for now. Done.
40 else if ("webkitCreateShadowRoot" in container)	33 var shadow = document.documentElement.webkitCreateShadowRoot();

41 container = container.webkitCreateShadowRoot();	34 shadow.appendChild(document.createElement("shadow"));
Wladimir Palant 2014/04/15 13:13:14 Please add a <shadow> element to our shadow root t Please add a <shadow> element to our shadow root to make sure any content defined by the document or other shadow roots is still in the DOM tree. Interestingly, Chrome doesn't seem to implement HTMLShadowElement.olderShadowRoot yet. This means that by adding one more shadow root with only the <shadow> element we can protect our style element from website accesses. Not sure whether this is worth doing. Sebastian Noack 2014/04/15 19:21:54 Done. Show quoted text On 2014/04/15 13:13:14, Wladimir Palant wrote: > Please add a <shadow> element to our shadow root to make sure any content > defined by the document or other shadow roots is still in the DOM tree. > > Interestingly, Chrome doesn't seem to implement > HTMLShadowElement.olderShadowRoot yet. This means that by adding one more shadow > root with only the <shadow> element we can protect our style element from > website accesses. Not sure whether this is worth doing. Done.
	35 shadow.appendChild(style);

42	36

43 elemhideElt = document.createElement("style");	37 for (var i = 0; i < selectors.length; i++)

44 elemhideElt.setAttribute("type", "text/css");	38 selectors[i] = "::-webkit-distributed(" + selectors[i] + ")";

45 container.appendChild(elemhideElt);	39 }
Wladimir Palant 2014/04/15 13:13:14 Will these styles affect the regular elements or o Will these styles affect the regular elements or only the shadow DOM? From http://www.w3.org/TR/shadow-dom/#styles it seems that the latter is the case. That was actually the reason why we dismissed using Shadow DOM for element hiding earlier. It seems that in order to have the styles apply to the document we have to create the shadow root on the document element and then use the ::-webkit-distributed() pseudo-element for each selector. I wonder whether we can measure the overhead (CPU time and memory) caused by this. Sebastian Noack 2014/04/15 19:21:54 You are right, it actually didn't worked with the Show quoted text On 2014/04/15 13:13:14, Wladimir Palant wrote: > Will these styles affect the regular elements or only the shadow DOM? From > http://www.w3.org/TR/shadow-dom/#styles it seems that the latter is the case. > That was actually the reason why we dismissed using Shadow DOM for element > hiding earlier. > > It seems that in order to have the styles apply to the document we have to > create the shadow root on the document element and then use the > ::-webkit-distributed() pseudo-element for each selector. I wonder whether we > can measure the overhead (CPU time and memory) caused by this. You are right, it actually didn't worked with the previous patch. I confused element hiding with element collapsing when I tested it. But after using the document element as shadow root, adding a <shadow> node to it and wrapping the selectors with ::-webkit-distributed() it works perfectly.
	40 else

	41 {

	42 // Try to insert the style into the <head> tag, inserting directly under the

	43 // document root breaks dev tools functionality:

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

	45 (document.head \|\| document.documentElement).appendChild(style);

	46 }

46	47

47 var elt = elemhideElt; // Use a local variable to avoid racing conditions	48 // WebKit apparently chokes when the selector list in a CSS rule is huge.

48 function setRules()	49 // So we split the elemhide selectors into groups.

	50 for (var i = 0; selectors.length > 0; i++)

49 {	51 {

50 if (!elt.sheet)	52 var selector = selectors.splice(0, SELECTOR_GROUP_SIZE).join(", ");
Sebastian Noack 2014/04/15 19:21:54 I also got rid of that code. The sheet property is I also got rid of that code. The sheet property is present as soon as you insert the element into the DOM. It works even with Safari 5.1, which is the oldest webkit browser I could find for download. Do you remember why we had that code in the first place? Wladimir Palant 2014/04/15 21:19:47 I didn't remember but hg blame points to https://a I didn't remember but hg blame points to https://adblockplus.org/forum/viewtopic.php?t=8834. As with any issues that cannot be reproduced, it is really hard to tell whether the issue is gone. However, it seems that this was running on a timeout even before the change in question - so older Chrome versions weren't initializing the element.sheet property immediately. Please verify that your code works in Chrome 19. You can get Chromium 19 (trunk build, right before the release branch was created) from https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.htm... (builds for other operating systems are available as well).
51 {	53 style.sheet.insertRule(selector + " { display: none !important; }", i);

52 // Stylesheet didn't initialize yet, wait a little longer

53 window.setTimeout(setRules, 0);

54 return;

55 }

56

57 // WebKit apparently chokes when the selector list in a CSS rule is huge.

58 // So we split the elemhide selectors into groups.

59 for (var i = 0, j = 0; i < selectors.length; i += SELECTOR_GROUP_SIZE, j++)

60 {

61 var selector = selectors.slice(i, i + SELECTOR_GROUP_SIZE).join(", ");

62 elt.sheet.insertRule(selector + " { display: none !important; }", j);

63 }

64 }	54 }

65 setRules();

66 }	55 }

67	56

68 var typeMap = {	57 var typeMap = {

69 "img": "IMAGE",	58 "img": "IMAGE",

70 "input": "IMAGE",	59 "input": "IMAGE",

71 "audio": "MEDIA",	60 "audio": "MEDIA",

72 "video": "MEDIA",	61 "video": "MEDIA",

73 "frame": "SUBDOCUMENT",	62 "frame": "SUBDOCUMENT",

74 "iframe": "SUBDOCUMENT"	63 "iframe": "SUBDOCUMENT"

75 };	64 };

(...skipping 46 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
122 ext.backgroundPage.sendMessage({type: "add-key-exception", token: attr});	111 ext.backgroundPage.sendMessage({type: "add-key-exception", token: attr});

123	112

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

125 }	114 }

126	115

127 // In Chrome 18 the document might not be initialized yet	116 // In Chrome 18 the document might not be initialized yet

128 if (document.documentElement)	117 if (document.documentElement)

129 init();	118 init();

130 else	119 else

131 window.setTimeout(init, 0);	120 window.setTimeout(init, 0);

LEFT	RIGHT