Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: safari/include.youtube.js

Issue 5607308285444096: Issue 1258 - Block ads in YouTube's HTML5 player on Safari (Closed)
Patch Set: Created Aug. 22, 2014, 2 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: safari/include.youtube.js
===================================================================
--- a/safari/include.youtube.js
+++ b/safari/include.youtube.js
@@ -22,11 +22,13 @@
if (!ext.backgroundPage.sendMessageSync({type: "get-domain-enabled-state"}).enabled)
return;
+ var badArgumentsRegex = /^((.*_)?(ad|ads|afv|adsense)(_.*)?|(ad3|st)_module|prerolls|interstitial|infringe|iv_cta_url)$/;
+
function rewriteFlashvars(flashvars)
{
var pairs = flashvars.split("&");
for (var i = 0; i < pairs.length; i++)
- if (/^((ad|afv|adsense)(_.*)?|(ad3|st)_module|prerolls|interstitial|infringe|iv_cta_url)=/.test(pairs[i]))
+ if (badArgumentsRegex.test(pairs[i].split("=")[0]))
pairs.splice(i--, 1);
return pairs.join("&");
}
@@ -66,21 +68,94 @@
player.parentNode.replaceChild(newPlayer, player);
}
+ function runInPage(fn, arg)
+ {
+ var script = document.createElement("script");
+ script.type = "application/javascript";
+ script.async = false;
+ script.textContent = "(" + fn + ")(" + arg + ");";
+ document.documentElement.appendChild(script);
+ document.documentElement.removeChild(script);
+ }
+
document.addEventListener("beforeload", function(event)
{
if ((event.target.localName == "object" || event.target.localName == "embed") && /:\/\/[^\/]*\.ytimg\.com\//.test(event.url))
patchPlayer(event.target);
}, true);
- // if history.pushState is available, YouTube uses the history API
- // when navigation from one video to another, and tells the flash
- // player with JavaScript which video and which ads to show next,
- // bypassing our flashvars rewrite code. So we disable
- // history.pushState before YouTube's JavaScript runs.
- var script = document.createElement("script");
- script.type = "application/javascript";
- script.async = false;
- script.textContent = "History.prototype.pushState = undefined;";
- document.documentElement.appendChild(script);
- document.documentElement.removeChild(script);
+ runInPage(function(badArgumentsRegex)
+ {
+ // if history.pushState is available, YouTube uses the history API
+ // when navigation from one video to another, and tells the flash
+ // player with JavaScript which video and which ads to show next,
+ // bypassing our flashvars rewrite code. So we disable
+ // history.pushState before YouTube's JavaScript runs.
+ History.prototype.pushState = undefined;
+
+ // the HMLT5 player is configured via ytplayer.config.args. We have
Wladimir Palant 2014/08/28 16:43:38 Nit: "The HTML5 player" (properly capitalized and
Sebastian Noack 2014/08/29 14:51:24 Done.
+ // to make sure that ad-related arguments are ignored as they are set.
+ var ytplayer = undefined;
Wladimir Palant 2014/08/28 16:43:38 What if ytplayer variable was set already? I'd rec
Sebastian Noack 2014/08/29 14:51:24 This hack only works anyway, if our JavaScript run
+ Object.defineProperty(window, "ytplayer",
+ {
+ get: function()
+ {
+ return ytplayer;
+ },
+ set: function(rawYtplayer)
+ {
+ if (typeof rawYtplayer != "object")
Wladimir Palant 2014/08/28 16:43:38 console.log(typeof null); This should be: if (
Sebastian Noack 2014/08/29 14:51:24 Thanks for reminding me that null is an object. I'
+ {
+ ytplayer = rawYtplayer;
+ return;
+ }
+
+ var config = undefined;
Wladimir Palant 2014/08/28 16:43:38 Here as well: what if the config property is alrea
Sebastian Noack 2014/08/29 14:51:24 This was handled by the loop below: for (var pr
+ ytplayer = {
+ get config()
+ {
+ return config;
+ },
+ set config(rawConfig)
+ {
+ if (typeof rawConfig != "object")
Wladimir Palant 2014/08/28 16:43:38 Here as well: please consider the possibility that
Sebastian Noack 2014/08/29 14:51:24 Done.
+ {
+ config = rawConfig;
+ return;
+ }
+
+ var args = undefined;
+ config = {
+ get args()
+ {
+ return args;
+ },
+ set args(rawArgs)
+ {
+ if (typeof rawArgs != "object")
+ {
+ args = rawArgs;
+ return;
+ }
+
+ args = {};
+ for (var arg in rawArgs)
+ {
+ if (!badArgumentsRegex.test(arg))
+ args[arg] = rawArgs[arg];
+ }
+ }
+ };
Wladimir Palant 2014/08/28 16:43:38 There is a recurring pattern here: you wrap object
Sebastian Noack 2014/08/29 14:51:24 Of course, I already considered that. However, the
+
+ for (var prop in rawConfig)
+ config[prop] = rawConfig[prop];
+ }
+ };
+
+ for (var prop in rawYtplayer)
+ ytplayer[prop] = rawYtplayer[prop];
Wladimir Palant 2014/08/28 16:43:38 I think that inheriting from rawYtplayer would be
Sebastian Noack 2014/08/29 14:51:24 Done. However, I'm not convinced that it is a bett
+ },
+ configurable: true
+ });
+ }, badArgumentsRegex);
})();
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld