Skip to content
Browse files

Fix console non-existence bug (this time with feeling)

  • Loading branch information...
1 parent 4caf81a commit 439a78ac54708ff339aaf1c31922d50458979714 @kriskowal committed May 14, 2012
Showing with 1 addition and 1 deletion.
  1. +1 −1 q.js
View
2 q.js
@@ -454,7 +454,7 @@ function isRejected(object) {
var rejections = [];
var errors = [];
-if (typeof window !== "undefined" && typeof window.console === "object") {
+if (typeof window !== "undefined" && window.console) {
// This promise library consumes exceptions thrown in handlers so
// they can be handled by a subsequent promise. The rejected
// promises get added to this array when they are created, and

4 comments on commit 439a78a

@michaelficarra

Still broken.

> window = null
null
> typeof window !== "undefined" && window.console
TypeError: Cannot read property 'console' of null

You don't want to access a property unless you know it's != null. My proposed fix would have been fine:

typeof window != "undefined" && window != null && window.console != null
@domenic
Collaborator

Broken is really overstating the case. This seems tantamount to protecting against Object.keys = Array.prototype.reduce = function () { throw new Error("pwned"); };.

@michaelficarra

Not really. If I'm unsure whether the variable is even declared, why would I be sure that it's non-null immediately after? Is it safe to assume that if a window reference is in scope, it's an object? If so, then feel free to leave it.

@kriskowal
Owner

I would rather not have to pay the price in bytes or complexity for a solution that will not be observably better in real usage. There is no system that protects against malice in previously run scripts. We might shoot for protecting against malice in subsequently run scripts.

Please sign in to comment.
Something went wrong with that request. Please try again.