Skip to content

Fix bad console assignment #241

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Fix bad console assignment #241

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/_patch/browserstack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
req.send(data);
}

var browserstack_console = console || window.console || {};
// Change some console method to capture the logs.
// 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 || {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't console and window.console in browser are same.
image

Copy link
Contributor Author

@Krinkle Krinkle Apr 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 defined

Previously 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.

browserstack_console.log = function () {
var args = BrowserStack.util.toArray(arguments).map(BrowserStack.util.inspect);
post('/_log/', { arguments: args }, function () {});
Expand All @@ -54,6 +58,7 @@
BrowserStack.worker_uuid = getParameterByName('_worker_key');

window.BrowserStack = BrowserStack;
// If the browser didn't have a console object (old IE), then this will create it.
// Otherwise this is a no-op as it will assign the same object it already held.
window.console = browserstack_console;
console = browserstack_console;
})();