Opened on 03/20/2018 at 04:14:23 PM
Closed on 09/04/2018 at 08:57:04 AM
#6507 closed change (rejected)
Inject style sheet proactively from background page
Reported by: | mjethani | Assignee: | mjethani |
---|---|---|---|
Priority: | Unknown | Milestone: | |
Module: | Platform | Keywords: | circumvention, externaldependency |
Cc: | sebastian, kzar | Blocked By: | #242, #5090 |
Blocking: | Platform: | Unknown / Cross platform | |
Ready: | no | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
Description (last modified by mjethani)
Background
The background page waits for the content script to send a message before it can inject the style sheet for element hiding. The message itself contains no information. Now with user style sheets (#5090, #242) the background page doesn't have to wait for the content script before injecting the style sheet, it can inject the style sheet preemptively if tabs.insertCSS with cssOrigin is supported.
The benefit of doing this mainly is for anti-circumvention.
Chromium does not run the content script on sandboxed about: frames, because they have a unique origin (this is questionable but it is the current behavior). Even after crrev.com/c/968681, it is not going to run content scripts in sandboxed data: frames.
In other words, if we have a filter ###ad and the following HTML code:
<iframe srcdoc="<div id='ad'>ANNOYING AD!!!</div>" sandbox></iframe>
That div#ad inside the <iframe> will not get hidden.
While currently neither Chromium nor Firefox will allow tabs.insertCSS on sandboxed about: and data: frames, it might be possible to convince both browsers to allow CSS (even if not JavaScript) to be injected into such frames.
What to change
Undecided.
Resolution
With the changes for snippets (#6539, #6782), we significantly refactored the code in lib/cssInjection.js (renaming it to lib/contentFiltering.js). The issue of injecting code as early as possible, as well as into about: and data: frames, is a lot more relevant for snippets than for element hiding. Since we have gained more knowledge and experience about this and are now considering the contentScripts API on Firefox (and possibly Chrome in the future) (#6826), it doesn't make sense to make this change any more.
Attachments (0)
Change History (7)
comment:1 Changed on 03/20/2018 at 04:15:11 PM by mjethani
comment:2 Changed on 03/20/2018 at 04:15:31 PM by mjethani
- Cc sebastian kzar added
comment:3 follow-up: ↓ 5 Changed on 03/20/2018 at 09:29:36 PM by sebastian
Sounds like a good idea generally. Besides the circumvention mitigation aspect, it also avoids a round trip from the content script to the background page. I'm just wondering about the timing; onCommitted is supposed to be emitted when the browser switched to showing the new document. If the CSS isn't already injected by then, elements might flash up for a short amount of time before they get hidden. At least we should make sure that this isn't getting worse than with our current approach.
comment:4 Changed on 03/22/2018 at 02:24:20 PM by mjethani
- Owner set to mjethani
comment:5 in reply to: ↑ 3 Changed on 04/06/2018 at 06:40:07 AM by mjethani
Replying to sebastian:
I'm just wondering about the timing; onCommitted is supposed to be emitted when the browser switched to showing the new document. If the CSS isn't already injected by then, elements might flash up for a short amount of time before they get hidden. At least we should make sure that this isn't getting worse than with our current approach.
I checked, and sometimes the onCommitted handler is called before the content script is loaded (rarely, but it happens) and usually it is called after. In any case, onCommitted comes before the content script's message reaches the background page. This should only make the situation better, not worse, at least for the best case (modern browsers with user style sheets support). Unfortunately sometimes onCommitted happens before the content script is even loaded, in that case we need to add a delay before we contact the content script. I think that's acceptable. It only happens rarely, and it affects only older browsers and DevTools.
I think we could use webNavigation.onCommitted to inject the style sheet early.