Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
GoogleCodeExporter opened this issue May 20, 2015 · 7 comments
Open

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

GoogleCodeExporter opened this issue May 20, 2015 · 7 comments

Comments

@GoogleCodeExporter
Copy link

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 [email protected] on 11 Apr 2011 at 4:10

@GoogleCodeExporter
Copy link
Author

So true! This is unbelievably frustrating for me as a user!

Original comment by [email protected] on 23 Dec 2011 at 9:57

@GoogleCodeExporter
Copy link
Author

I believe this quick-and-dirty patch should improve the situation.

Original comment by [email protected] on 7 Feb 2012 at 12:49

Attachments:

@GoogleCodeExporter
Copy link
Author

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 [email protected] on 29 May 2012 at 6:00

@GoogleCodeExporter
Copy link
Author

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 [email protected] on 30 May 2012 at 8:58

@GoogleCodeExporter
Copy link
Author

We're seeing these exceptions on github.com as well. (I'm a GitHub engineer.)

Original comment by [email protected] on 6 Mar 2014 at 3:40

@GoogleCodeExporter
Copy link
Author

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 [email protected] on 27 Dec 2014 at 12:08

@GoogleCodeExporter
Copy link
Author

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

Original comment by [email protected] on 5 Mar 2015 at 8:01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant