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

Already on GitHub? Sign in to your account

IE8 console.group error #1206

Closed
amid2887 opened this Issue Sep 13, 2012 · 7 comments

Comments

Projects
None yet
5 participants

JavaScript wrapper for console methods ( in the plugins.js ) not works perfectly..
If we use for example console.group method with this wrapper it will be js error that browser has no support group method but support console method.

Tested: windows 7x32. ie9 (ie8 mode)

Contributor

devinrhode2 commented Sep 18, 2012

I attempted to replicate this issue in IE8 and IE9 with browser stack and this test page, but could not replicate the issue.

I should note however, that I could not select Tools > Compatibility View in browserstack's IE9, so that does leave the possibility that the code is failing only in IE9's IE8 mode.

@amid2887, could you double check if it's failing only for IE9's IE8 mode?

Feel feel to use my test page. When you load it in IE8-9, the expected behavior is this sequence of popups:

  1. "attempting console.group"
  2. "console.group failed, running stubs and trying again"
  3. "successfully ran console.group after method stubs."

You can also access this fiddle of my test page's javascript: http://jsfiddle.net/DevinRhode2/DLtPd/2/

However

I did find an error with this code on an older version of Opera 10 on windows. This browser version apparently defines console and/or console.log, but not console.group (and certainly others)

Therefore, this code assumes that if console.log is available, all the console methods are good to go, which is incorrect.

For reference, here's the current javascript in question:

// Avoid `console` errors in browsers that lack a console.
if (!(window.console && console.log)) {
    (function() {
        var noop = function() {};
        var methods = ['assert', 'clear', 'count', 'debug', 'dir', 'dirxml', 'error', 'exception', 'group', 'groupCollapsed', 'groupEnd', 'info', 'log', 'markTimeline', 'profile', 'profileEnd', 'markTimeline', 'table', 'time', 'timeEnd', 'timeStamp', 'trace', 'warn'];
        var length = methods.length;
        var console = window.console = {};
        while (length--) {
            console[methods[length]] = noop;
        }
    }());
}

Here is my javascript that fixes the above issues:

// Avoid `console` errors in browsers that lack a console.
(function consoleStub() {
    if (!window.console) {
        window.console = {}; //moved line for clarity
    }

    //This code always executes.
    var console = window.console; //moar opera errors around this - console wasn't defined below, yes it wasn't resolving to window.console for some reason.
    var noop = function noop() {};
    var methods = ['assert', 'clear', 'count', 'debug', 'dir', 'dirxml', 
                   'error', 'exception', 'group', 'groupCollapsed',
                   'groupEnd', 'info', 'log', 'markTimeline', 'profile',
                   'profileEnd', 'markTimeline', 'table', 'time',
                   'timeEnd', 'timeStamp', 'trace', 'warn'];
    var length = methods.length;
    while (length--) {
        //only add method if it's not already defined.
        if (!console[methods[length]]) {
            console[methods[length]] = noop;
        }
    }
}());

Let me know if I should submit a pull request of with this change. I'd love to be able to say I contributed to boilerplate!

Contributor

devinrhode2 commented Sep 18, 2012

Related to Dropping legacy browser support, I'd be fine with keeping the line: var console = window.console = {};, I think this was only with Opera 10, however I ran out of browser stack minutes so I don't know (or care enough) about other browsers.

However, the console.log logic change is a matter or having completely correct code, I think we should definitely have that logic change.

Checked your's test page. Here some results:

  • Windows 7 x64, Windows 7 x32
    • Internet Explorer 9 ( IE9 mode, IE8 mode, IE7 mode ):
      1. "attempting console.group";
      2. "console.group failed, running stubs and trying again";
      3. "consoleStubber stub code run fine";
      4. "successfully ran console.group after method stubs.";

Also Windows 8 x64 has the same result in IE10 ( with different modes ).

Also method _markTimeline_ defined twice... Is it typing??

P.S. I'm trying to build some primitive test page http://jsfiddle.net/CR2zX/2/.
P.S.S. If you need some tests in any else browser, tell me and I'll try to help.

Contributor

devinrhode2 commented Sep 18, 2012

Those are the correct popups with my new console stub code, so it got the job done with no errors.

To test the old console stub code, I made a duplicate of that test page that uses the current stub code: old_stub.php

If you get the same popups on both pages in all scenarios, then I don't know where your original error came from, and the original stub code always works fine for all these versions of IE.

However, this code can still be made more robust by not assuming the full console api is available if console.log is available.

Owner

mathiasbynens commented Sep 20, 2012

Also method markTimeline defined twice...

Yup, that’s a typo. Good catch!

Owner

drublic commented Sep 20, 2012

Tests

On Windows 7 IE9 I get the following results with the old stub:

  1. "attempting console.group"
  2. "console.group failed, running stubs and trying again"
  3. "consoleStubber stub code run fine"
  4. "successfully ran console.group after method stubs."

The code with the new stub (index.php) fails for me:

1."attempting console.group"
2. "console.group failed, running stubs and trying again"
3. "consoleStubber stub code run fine"
4. "stub methods actually failed"

So the last (//WILL EXECUTE) try…catch fails in IE9 (IE7-IE9 mode) for me.

I guess this means we need to handle this method in some other way.

Possible Solution

I think the method @devinrhode2 describes would be a good way to go.
I've updated this function a bit:

// Avoid `console` errors in browsers that lack a console.
(function() {
        var noop = function() {};
        var methods = ['assert', 'clear', 'count', 'debug', 'dir', 'dirxml', 'error', 'exception', 'group', 'groupCollapsed', 'groupEnd', 'info', 'log', 'markTimeline', 'profile', 'profileEnd', 'markTimeline', 'table', 'time', 'timeEnd', 'timeStamp', 'trace', 'warn'];
        var length = methods.length;
        var console = window.console || {};

        while (length--) {
            if (!console[methods[length]]) {
                console[methods[length]] = noop;
            }
        }
}());

Works for me in IE9 (IE7-IE9 mode). Couldn't install Opera 10 and I actually think we should not care about this browser anymore.

Contributor

devinrhode2 commented Sep 20, 2012

@drublic I like your code better ;)

I sent a pull request with these changes :)

For anyone looking to test the differences in browsers, /test2/old_stub.php contains the previous stub code, and /test2/new_stub.php contains the new stub code. Theindex.phppage that drublic was referring to was properly renamedold_stub.php(where his test failed)

View source and hack on it as you please

@necolas necolas closed this in 578f377 Sep 22, 2012

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