sanderv32 / betterpopupblocker

better popup blocker
Other
0 stars 2 forks source link

Don't set javascript methods to "null" #1

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Hi,

I'm working for a big social networking site where we log javascript exceptions 
happening in the user's browser.
In the past weeks one strange js error caught our attention:
"Uncaught TypeError: Property 'scrollTo' of object [object DOMWindow] is not a 
function"

The strange thing about this error is that it only appeared in Chrome.
After some investigation I found this extension which seems to be the cause for 
this error.

In line #100 of blockStart.js a bunch of javascript functions (including 
window.scrollTo) is set to 'null'.
I can fully understand that unsetting those variables makes sense.

The only problem I have is that the type of the variable is changed which leads 
to javascript exceptions and prevents other "non-annoying" functionality from 
being executed.

Imagine this piece of code:
doSomething();
window.scrollTo(0, 0); // <-- not a function, therefore causes a js exception
doSomethingElseImportant(); // <-- will never be executed when window.scrollTo 
is null

If the extension is enabled the "doSomethingElseImportant()" will never be 
executed since the window.scrollTo method throws a js error which stops 
javascript execution at that point.

Would be really helpful if the "Better PopUp Blocker" would preserve the 
original type.
So instead of setting all those javascript functions to null, it would make 
more sense to set them to an empty function. "window.scrollTo=function(){};"

Thanks!

Christopher

Original issue reported on code.google.com by christop...@googlemail.com on 11 Apr 2011 at 4:10

GoogleCodeExporter commented 9 years ago
So true! This is unbelievably frustrating for me as a user!

Original comment by idle...@gmail.com on 23 Dec 2011 at 9:57

GoogleCodeExporter commented 9 years ago
I believe this quick-and-dirty patch should improve the situation.

Original comment by drdae...@gmail.com on 7 Feb 2012 at 12:49

Attachments:

GoogleCodeExporter commented 9 years ago
Well, setting getSelection to undefined still breaks some sites and extensios 
(namely, Evernote's Clearly). The following should work better: 
"document.getSelection=window.getSelection=function(){return new Object};"

A proper way, I believe, should be to proxy calls, but replace any unsafe 
methods on resulting DOMSelection object with dummy functions.

Original comment by drdae...@gmail.com on 29 May 2012 at 6:00

GoogleCodeExporter commented 9 years ago
It's definitely a better approach. The only problem I can see is with naive 
feature detection:

window.scrollTo = function () { return new Object }
window.scrollTo && doThingsThatUseScrollTo(); //happens

window.scrollTo = null;
window.scrollTo && doThingsTheUseScrollTo(); //never happens

However I can't see this being a problem because all the functions being nulled 
by blockstart.js are being discarded intentionally.

Original comment by muld...@gmail.com on 30 May 2012 at 8:58

GoogleCodeExporter commented 9 years ago
We're seeing these exceptions on github.com as well. (I'm a GitHub engineer.)

Original comment by adam.ro...@gmail.com on 6 Mar 2014 at 3:40

GoogleCodeExporter commented 9 years ago
Just wanted to take a moment to thank “blocker” authors for screwing 
websites for a lot of users who think websites are broken.

Original comment by dan.abra...@gmail.com on 27 Dec 2014 at 12:08

GoogleCodeExporter commented 9 years ago
Rage-starred, for what little it's worth. How was this ever a good idea?

Original comment by isaacsal...@gmail.com on 5 Mar 2015 at 8:01