Issue 4191 - Restore rules for reinjected stylesheets

Issue 4191 - Restore rules for reinjected stylesheets

(Note: built on top of the WebSocket changes that are still under review.)

[kzar, 2016-07-29 21:35:38]

Patch Set 1

[Sebastian Noack, 2016-08-10 13:21:50]

How do they remove those rules in the first place? Is there no way to prevent
that (like we already do by wrapping some DOM APIs)? Also, if they (however)
manage to remove those rules, couldn't they do the same without triggering the
MutationObserver, rendering this workaround ineffective?

[kzar, 2016-08-11 11:04:41]

Patch Set 2: Rebased.

As discussed in IRC simply removing the style element from the DOM seems to
trash the style's sheet along with it. Try this to see for yourself:

let style = document.body.parentNode.shadowRoot.styleSheets[0].ownerNode;
let parent = style.parentNode;

style.sheet.rules.length;

parent.removeChild(style);
style.sheet.rules.length;

[kzar, 2016-08-11 12:24:54]

Patch Set 3 : Prevent access to our shadowRoot

[Sebastian Noack, 2016-08-11 12:38:57]

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (left):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:365: style.id = id;
Since the id is only used once (for the WebSocket) now, mind moving the code
creating it, from the global scope to where it is used.

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:471: return shadow === ourShadowRoot ? null : shadow;
Nit: As per Mozilla's coding practices, we prefer === over ==.

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:474: }, "");
Nit: I'd rather pass null, than creating an empty string.

[Sebastian Noack, 2016-08-11 12:42:38]

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:471: return shadow === ourShadowRoot ? null : shadow;
On 2016/08/11 12:38:57, Sebastian Noack wrote:
> Nit: As per Mozilla's coding practices, we prefer === over ==.

(The other way around, I meant, == is preferred over ===)

[Sebastian Noack, 2016-08-11 12:48:11]

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:461: runInPage(function()
You have to pass the document to runInPage(), to make sure that it gets injected
into the right document.

[kzar, 2016-08-11 12:48:18]

Patch Set 4 : Addressed nits

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (left):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:365: style.id = id;
On 2016/08/11 12:38:57, Sebastian Noack wrote:
> Since the id is only used once (for the WebSocket) now, mind moving the code
> creating it, from the global scope to where it is used.

Done.

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:471: return shadow === ourShadowRoot ? null : shadow;
On 2016/08/11 12:38:57, Sebastian Noack wrote:
> Nit: As per Mozilla's coding practices, we prefer === over ==.

Done.

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:474: }, "");
On 2016/08/11 12:38:57, Sebastian Noack wrote:
> Nit: I'd rather pass null, than creating an empty string.

Done.

[kzar, 2016-08-11 13:04:47]

Patch Set 5 : runInDocument

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29348917/diff/29349653/include.preload.js#...
include.preload.js:461: runInPage(function()
On 2016/08/11 12:48:11, Sebastian Noack wrote:
> You have to pass the document to runInPage(), to make sure that it gets
injected
> into the right document.

Done.

[Sebastian Noack, 2016-08-11 13:12:03]

LGTM