Index: lib/scriptInjection.js |
=================================================================== |
rename from lib/cssInjection.js |
rename to lib/scriptInjection.js |
--- a/lib/cssInjection.js |
+++ b/lib/scriptInjection.js |
@@ -10,39 +10,43 @@ |
* but WITHOUT ANY WARRANTY; without even the implied warranty of |
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
* GNU General Public License for more details. |
* |
* You should have received a copy of the GNU General Public License |
* along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. |
*/ |
-/** @module cssInjection */ |
+/** @module scriptInjection */ |
kzar
2018/07/19 12:24:19
Be careful that Mercurial knows you've renamed lib
kzar
2018/07/19 12:24:20
Like I mentioned on one of the other reviews, I do
Manish Jethani
2018/07/19 12:55:24
Fair enough.
Since we're referring to it as "cont
Manish Jethani
2018/07/19 12:55:25
Thanks, I'll try to do this correctly.
kzar
2018/07/19 13:28:55
Sounds good.
Manish Jethani
2018/07/19 14:04:35
Done.
|
"use strict"; |
const {RegExpFilter} = require("../adblockpluscore/lib/filterClasses"); |
const {ElemHide} = require("../adblockpluscore/lib/elemHide"); |
const {ElemHideEmulation} = require("../adblockpluscore/lib/elemHideEmulation"); |
+const {Snippets, compileScript} = require("../adblockpluscore/lib/snippets"); |
const {checkWhitelisted} = require("./whitelisting"); |
const {extractHostFromFrame} = require("./url"); |
const {port} = require("./messaging"); |
const {HitLogger} = require("./hitLogger"); |
const info = require("info"); |
// Chromium's support for tabs.removeCSS is still a work in progress and the |
// API is likely to be different from Firefox's; for now we just don't use it |
// at all, even if it's available. |
// See https://crbug.com/608854 |
const styleSheetRemovalSupported = info.platform == "gecko"; |
const selectorGroupSize = 1024; |
let userStyleSheetsSupported = true; |
+let snippetsLibrarySource = ""; |
+let executableCode = new Map(); |
+ |
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 |
@@ -90,17 +94,17 @@ |
userStyleSheetsSupported = false; |
// For other errors, we simply return false to indicate failure. |
// |
// One common error that occurs frequently is when a frame is not found |
// (e.g. "Error: No frame with id 574 in tab 266"), which can happen when |
// the code in the parent document has removed the frame before the |
// background page has had a chance to respond to the content script's |
- // "elemhide.getSelectors" message. We simply ignore such errors, because |
+ // "content.applyFilters" message. We simply ignore such errors, because |
// otherwise they show up in the log too often and make debugging |
// difficult. |
// |
// Also note that the missing frame error is thrown synchronously on |
// Firefox, while on Chromium it is an asychronous promise rejection. In |
// the latter case, we cannot indicate failure to the caller, but we still |
// explicitly ignore the error. |
return false; |
@@ -155,35 +159,84 @@ |
// style sheet now. |
if (oldStyleSheet && oldStyleSheet != styleSheet) |
removeStyleSheet(tabId, frameId, oldStyleSheet); |
frame.injectedStyleSheets.set(groupName, styleSheet); |
return true; |
} |
-port.on("elemhide.getSelectors", (message, sender) => |
+function getExecutableCode(script) |
+{ |
+ let code = executableCode.get(script); |
+ if (code) |
+ return code; |
+ |
+ code = compileScript(script, [snippetsLibrarySource]); |
+ |
+ executableCode.set(script, code); |
+ return code; |
+} |
+ |
+function executeScript(script, tabId, frameId) |
+{ |
+ try |
+ { |
+ browser.tabs.executeScript(tabId, { |
+ code: getExecutableCode(script), |
+ frameId, |
+ matchAboutBlank: true, |
+ runAt: "document_start" |
+ }) |
+ .catch(error => |
+ { |
+ // Sometimes a frame is added and removed very quickly, in such cases we |
+ // simply ignore the error. |
+ if (error.message == "The frame was removed.") |
+ return; |
+ |
+ throw error; |
+ }); |
+ } |
+ catch (error) |
kzar
2018/07/19 12:24:21
So we re-throw the exception, catch it again, then
Manish Jethani
2018/07/19 12:55:24
No, the error thrown from the .catch() handler is
kzar
2018/07/19 13:28:55
OK.
Manish Jethani
2018/07/19 14:04:34
It's the "The frame was removed." error.
kzar
2018/07/19 14:52:34
That's the above catch. I thought we were talking
Manish Jethani
2018/07/19 15:33:38
Ah, I see what you mean. Yeah, I probably just add
kzar
2018/07/19 15:37:45
Yea, ideally there would be an entry in the develo
Manish Jethani
2018/07/19 15:44:05
Can we simply log the error?
catch (error)
{
Manish Jethani
2018/07/19 15:54:08
OK, so when a snippet throws an error that error i
kzar
2018/07/19 16:07:40
Alright, fair enough.
|
+ { |
+ } |
+} |
+ |
+port.on("content.applyFilters", (message, sender) => |
{ |
let selectors = []; |
let emulatedPatterns = []; |
let trace = HitLogger.hasListener(sender.page.id); |
let inline = !userStyleSheetsSupported; |
+ let {elemhide = false, snippets = false} = message.filterTypes || |
kzar
2018/07/19 12:24:19
Is this syntax supported by the browsers we suppor
Manish Jethani
2018/07/19 12:55:25
Yeah, that's just object destructuring with defaul
Manish Jethani
2018/07/19 14:04:35
Done.
|
+ {elemhide: true, snippets: true}; |
+ |
if (!checkWhitelisted(sender.page, sender.frame, null, |
- RegExpFilter.typeMap.DOCUMENT | |
- RegExpFilter.typeMap.ELEMHIDE)) |
+ RegExpFilter.typeMap.DOCUMENT)) |
{ |
let hostname = extractHostFromFrame(sender.frame); |
- let specificOnly = checkWhitelisted(sender.page, sender.frame, null, |
- RegExpFilter.typeMap.GENERICHIDE); |
+ |
+ if (snippets) |
+ { |
+ for (let script of Snippets.getScriptsForDomain(hostname)) |
+ executeScript(script, sender.page.id, sender.frame.id); |
+ } |
- selectors = ElemHide.getSelectorsForDomain(hostname, specificOnly); |
+ if (elemhide && !checkWhitelisted(sender.page, sender.frame, null, |
+ RegExpFilter.typeMap.ELEMHIDE)) |
kzar
2018/07/19 12:24:20
It kind of sucks we have to call checkWhitelisted
Manish Jethani
2018/07/19 12:55:24
#@# exception will be applied within Snippets.getS
kzar
2018/07/19 13:28:57
Alright, fair enough I guess.
|
+ { |
+ let specificOnly = checkWhitelisted(sender.page, sender.frame, null, |
kzar
2018/07/19 12:24:18
If we don't check for generichide/genericblock for
Manish Jethani
2018/07/19 12:55:25
Currently there are no such checks. I'll mention t
kzar
2018/07/19 13:28:54
Yea, I definitely think so. Please could you open
Manish Jethani
2018/07/19 14:04:35
Done.
https://issues.adblockplus.org/ticket/6797
kzar
2018/07/19 14:52:34
Thanks
|
+ RegExpFilter.typeMap.GENERICHIDE); |
+ selectors = ElemHide.getSelectorsForDomain(hostname, specificOnly); |
- for (let filter of ElemHideEmulation.getRulesForDomain(hostname)) |
- emulatedPatterns.push({selector: filter.selector, text: filter.text}); |
+ for (let filter of ElemHideEmulation.getRulesForDomain(hostname)) |
+ emulatedPatterns.push({selector: filter.selector, text: filter.text}); |
+ } |
} |
if (!inline && !updateFrameStyles(sender.page.id, sender.frame.id, |
selectors, "standard")) |
{ |
inline = true; |
} |
@@ -201,8 +254,15 @@ |
return response; |
}); |
port.on("elemhide.injectSelectors", (message, sender) => |
{ |
updateFrameStyles(sender.page.id, sender.frame.id, message.selectors, |
message.groupName, message.appendOnly); |
}); |
+ |
+fetch(browser.extension.getURL("/snippets.js"), {cache: "no-cache"}) |
kzar
2018/07/19 12:24:20
What if fetch throws?
Manish Jethani
2018/07/19 12:55:24
I don't see how it could throw in practice if the
kzar
2018/07/19 13:28:55
Acknowledged.
|
+.then(response => response.ok ? response.text() : "") |
kzar
2018/07/19 12:24:21
Nit: We normally put some indentation before the `
Manish Jethani
2018/07/19 12:55:25
I thought when we break foo.bar.lambda() then (1)
kzar
2018/07/19 13:28:56
Well, with your example it would be more like this
|
+.then(text => |
kzar
2018/07/19 12:24:20
Why not just do the assignment in the previous fun
Manish Jethani
2018/07/19 12:55:26
response.text() returns a promise, which then reso
kzar
2018/07/19 13:28:54
Ah right, the indirection makes sense then, fair e
|
+{ |
+ snippetsLibrarySource = text; |
+}); |