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

Fixed the console stubbing utility. #1229

Closed
wants to merge 1 commit into from
Closed

Fixed the console stubbing utility. #1229

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 3, 2012

As it is being discussed in http://git.io/qEe7WA, the console stubbing utility is not working as intended (it is throwing, for example, in IE6). The associated commit, which is essentially a shortened version of the patch provided by @bgrins in the diff discussion, fixes this problem by actually exposing a global console object (which will either be the original console method, if defined, or a dummy {} object).

@necolas
Copy link
Member

necolas commented Oct 3, 2012

@mathiasbynens Got any time to look over what needs doing with this function?

@paulirish
Copy link
Member

legit. 👍

drublic pushed a commit that referenced this pull request Oct 14, 2012
Expose `console` object to global scope in IE6 and IE7

Reference: gh-1229
@drublic drublic closed this in b2ab421 Oct 14, 2012
@drublic
Copy link
Member

drublic commented Oct 14, 2012

Thanks for the PR, @bmcustodio.
This code works for me in IE6 and IE7. Here is my test.

I've added this PR and the other improvements outlined in the comments in 578f377 on branch console-stub.
Can you please verify that the code in this branch is working for you?

@drublic drublic reopened this Oct 14, 2012
@ghost
Copy link
Author

ghost commented Oct 14, 2012

@drublic: Yep, the code in this branch works just fine under IE6 and IE7. I have ran your test as well and everything seems smooth.

@drublic drublic closed this in 750bf0e Oct 14, 2012
@drublic drublic reopened this Oct 14, 2012
@drublic
Copy link
Member

drublic commented Oct 14, 2012

Thanks for looking into that again, @bmcustodio!
@mathiasbynens does this look good for you?

@necolas
Copy link
Member

necolas commented Oct 19, 2012

Thanks guys. Merged into master now.

@necolas necolas closed this Oct 19, 2012
@drublic drublic mentioned this pull request Nov 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants