Issue 4351 - Check History variable exists

Issue 4351 - Check History variable exists

[kzar, 2016-10-12 15:48:46]

Patch Set 1

(I've not taken the time to reproduce this and therefore test the fix.)

[Sebastian Noack, 2016-10-12 15:51:58]

https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
File safari/include.youtube.js (right):

https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
safari/include.youtube.js:100: if (typeof History != "undefined")
It seems that the History object is supported since Safari 5:
https://developer.mozilla.org/en-US/docs/Web/API/History

I suggest to first reproduce this issue, before taking any premature actions.

[kzar, 2016-10-12 15:53:31]

On 2016/10/12 15:51:58, Sebastian Noack wrote:
>
https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
> File safari/include.youtube.js (right):
> 
>
https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
> safari/include.youtube.js:100: if (typeof History != "undefined")
> It seems that the History object is supported since Safari 5:
> https://developer.mozilla.org/en-US/docs/Web/API/History
> 
> I suggest to first reproduce this issue, before taking any premature actions.

Normally I would agree of course but from the issue: "An exception
ReferenceError: Can't find variable: History is being ​displayed in the console.
Only when Adblock Plus is installed and only for YouTube."
https://usercontent.irccloud-cdn.com/file/wR7B8Bm7/Safari.jpg

[Sebastian Noack, 2016-10-12 18:03:35]

LGTM

https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
File safari/include.youtube.js (right):

https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
safari/include.youtube.js:100: if (typeof History != "undefined")
On 2016/10/12 15:53:31, kzar wrote:
> On 2016/10/12 15:51:58, Sebastian Noack wrote:
> >
>
https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
> > File safari/include.youtube.js (right):
> > 
> >
>
https://codereview.adblockplus.org/29356615/diff/29356616/safari/include.yout...
> > safari/include.youtube.js:100: if (typeof History != "undefined")
> > It seems that the History object is supported since Safari 5:
> > https://developer.mozilla.org/en-US/docs/Web/API/History
> > 
> > I suggest to first reproduce this issue, before taking any premature
actions.
> 
> Normally I would agree of course but from the issue: "An exception
> ReferenceError: Can't find variable: History is being ​displayed in the
console.
> Only when Adblock Plus is installed and only for YouTube."
> https://usercontent.irccloud-cdn.com/file/wR7B8Bm7/Safari.jpg

Fair enough, lets give it a try. But please check with Robert, whether this
actually fixes blocking of YouTube ads, and revert this change if it does not.

While it should avoid that exception, it is still unclear why the History object
isn't there in the first place. There might be a larger issue, which causes more
problems down the code path. Or the History object might just not be visible to
our script for some reason, but to YouTube's script in which case ads will be
back when clicking on another video.

[kzar, 2016-10-12 18:22:42]

On 2016/10/12 18:03:35, Sebastian Noack wrote:
> Fair enough, lets give it a try. But please check with Robert, whether this
> actually fixes blocking of YouTube ads, and revert this change if it does not.

Yea, that's fair. I've asked Robert to test it and mentioned that if it doesn't
help for him I'll revert the change.

> While it should avoid that exception, it is still unclear why the History
object
> isn't there in the first place. There might be a larger issue, which causes
more
> problems down the code path. Or the History object might just not be visible
to
> our script for some reason, but to YouTube's script in which case ads will be
> back when clicking on another video.

Right, but to be honest since this is an old version of a barely supported
platform I'd be happy to close as wontfix if the quick fix doesn't help.