Fix bad console assignment #241
Conversation
Follows-up 826d2c7. Before that commit, the reason global "console" was globbered is that the `var console` part of the inner `var console = {}` assignment was hosted by the JavaScript engine. Thus the variable always existed in the local scope as type "undefined", and so the conditional check never saw the real console, and always created a custom one. Thus it always deleted the original console reference, as well as any other it contained. In commit 826d2c7, this was incorrectly fixed by assigning the unchecked expression referring to `console`, thus no longer having a fallback to `{}` in older browsers, because an unchecked reference like that throws an Uncaught ReferenceError. As a result, browserstack-runner was unable to run in IE 9 or older. Fixes browserstack#161. Fixes browserstack#164. Fixes browserstack#212.
Follows-up 826d2c7. Before that commit, the reason global "console" was globbered is that the `var console` part of the inner `var console = {}` assignment was hosted by the JavaScript engine. Thus the variable always existed in the local scope as type "undefined", and so the conditional check never saw the real console, and always created a custom one. Thus it always deleted the original console reference, as well as any other it contained. In commit 826d2c7, this was incorrectly fixed by assigning the unchecked expression referring to `console`, thus no longer having a fallback to `{}` in older browsers, because an unchecked reference like that throws an Uncaught ReferenceError. As a result, browserstack-runner was unable to run in IE 9 or older. Fixes browserstack#161. Fixes browserstack#164. Fixes browserstack#212. Ref browserstack#241.
| // This must not replace the console object itself so that other console methods | ||
| // from the browser or added by the tested application remain unaffected. | ||
| // https://github.com/browserstack/browserstack-runner/pull/199 | ||
| var browserstack_console = window.console || {}; |
There was a problem hiding this comment.
Can't we copy the prototype and update the concerned method only so that we do not lose over other methods as well as retain the current logic.
There was a problem hiding this comment.
Isn't console and window.console in browser are same.
There was a problem hiding this comment.
Yes, in JavaScript the global scope of variables is reflected into an object called window where each global variable is a property. There are some exceptions, but in general, yes, when a variable actually exists and is defined as global, then foo === window.foo.
The problem, and the reason this code exists, is with when the variable does not exist, such as in old Internet Explorer.
Try this one:
window.thisdontexist
//> undefined
thisdontexist
//> Uncaught ReferenceError: thisdontexist is not definedPreviously this code used console. Which means that since the release of 826d2c7, browserstack-runner was completely broken in IE6-8, because after this ReferenceError: console is not defined, the program is terminated, the callback never installed, the worker unreachable, and the test results never arrive / timeout.
var x = window.console || {};
// Modern browser > Console
// IE 6-8 > {}var x = console || {};
// Modern browser > Console
// IE 6-8 > Uncaught ReferenceError: console is not defined / Program terminated.
Follows-up 826d2c7.
Before that commit, the reason global "console" was globbered is that the
var consolepart of the innervar console = {}assignment was hosted bythe JavaScript engine. Thus the variable always existed in the local scope as
type "undefined", and so the conditional check never saw the real console, and
always created a custom one. Thus it always deleted the original console reference,
as well as any other it contained.
In commit 826d2c7, this was incorrectly fixed by assigning the unchecked
expression referring to
console, thus no longer having a fallback to{}in olderbrowsers, because an unchecked reference like that throws an Uncaught ReferenceError.
As a result, browserstack-runner was unable to run in IE 9 or older.
Fixes #161.
Fixes #164.
Fixes #212.