@misteroneill improved Logging for IE < 11. closes #3356

CHANGELOG.md

@@ -18,6 +18,7 @@ CHANGELOG
 * @vit-koumar updated Flash tech to return Infinity from duration instead of -1 ([view](https://github.com/videojs/video.js/pull/3128))
 * @alex-phillips added ontextdata to Flash tech ([view](https://github.com/videojs/video.js/pull/2748))
 * @MattiasBuelens updated components to use durationchange only ([view](https://github.com/videojs/video.js/pull/3349))
+* @misteroneill improved Logging for IE < 11 ([view](https://github.com/videojs/video.js/pull/3356))
 
 --------------------
 

src/js/utils/browser.js

@@ -60,6 +60,9 @@ export const IS_FIREFOX = (/Firefox/i).test(USER_AGENT);
 export const IS_EDGE = (/Edge/i).test(USER_AGENT);
 export const IS_CHROME = !IS_EDGE && (/Chrome/i).test(USER_AGENT);
 export const IS_IE8 = (/MSIE\s8\.0/).test(USER_AGENT);
+export const IE_VERSION = (function(result){
+  return result && parseFloat(result[1]);
+})((/MSIE\s(\d+)\.\d/).exec(USER_AGENT));
 
 export const TOUCH_ENABLED = !!(('ontouchstart' in window) || window.DocumentTouch && document instanceof window.DocumentTouch);
 export const BACKGROUND_SIZE_SUPPORTED = 'backgroundSize' in document.createElement('video').style;

src/js/utils/log.js

@@ -2,78 +2,93 @@
  * @file log.js
  */
 import window from 'global/window';
+import {IE_VERSION} from './browser';
 
 /**
- * Log plain debug messages
+ * Log messages to the console and history based on the type of message
+ *
+ * @param  {String} type
+ *         The name of the console method to use.
+ * @param  {Array} args
+ *         The arguments to be passed to the matching console method.
+ * @param  {Boolean} [stringify]
+ *         By default, only old IEs should get console argument stringification,
+ *         but this is exposed as a parameter to facilitate testing.
  */
-const log = function(){
-  _logType(null, arguments);
+export const logByType = (type, args, stringify = !!IE_VERSION && IE_VERSION < 11) => {
+  const console = window.console;
+
+  // If there's no console then don't try to output messages, but they will
+  // still be stored in `log.history`.
+  //
+  // Was setting these once outside of this function, but containing them
+  // in the function makes it easier to test cases where console doesn't exist
+  // when the module is executed.
+  const fn = console && console[type] || function(){};
+
+  if (type !== 'log') {
+
+    // add the type to the front of the message when it's not "log"
+    args.unshift(type.toUpperCase() + ':');
+  }
+
+  // add to history
+  log.history.push(args);
+
+  // add console prefix after adding to history
+  args.unshift('VIDEOJS:');
+
+  // Old IE versions do not allow .apply() for some/all console method(s). And
+  // IEs previous to 11 log objects uselessly as "[object Object]"; so, JSONify
+  // objects and arrays for those less-capable browsers.
+  if (!fn.apply || stringify) {
+    fn(args.map(a => {
+      if (a && typeof a === 'object' || Array.isArray(a)) {
+        try {
+          return JSON.stringify(a);
+        } catch (x) {}
+      }
+
+      // Cast to string before joining, so we get null and undefined explicitly
+      // included in output (as we would in a modern console).
+      return String(a);
+    }).join(' '));
+
+  // Default hanlding for modern consoles.
+  } else {
+    fn.apply(console, args);
+  }
 };
 
+/**
+ * Log plain debug messages
+ *
+ * @function log
+ */
+function log(...args) {
+  logByType('log', args);
+}
+
 /**
  * Keep a history of log messages
+ *
  * @type {Array}
  */
 log.history = [];
 
 /**
  * Log error messages
+ *
+ * @method error
  */
-log.error = function(){
-  _logType('error', arguments);
-};
+log.error = (...args) => logByType('error', args);
 
 /**
  * Log warning messages
- */
-log.warn = function(){
-  _logType('warn', arguments);
-};
-
-/**
- * Log messages to the console and history based on the type of message
  *
- * @param  {String} type The type of message, or `null` for `log`
- * @param  {Object} args The args to be passed to the log
- * @private
- * @method _logType
+ * @method warn
  */
-function _logType(type, args){
-  // convert args to an array to get array functions
-  let argsArray = Array.prototype.slice.call(args);
-  // if there's no console then don't try to output messages
-  // they will still be stored in log.history
-  // Was setting these once outside of this function, but containing them
-  // in the function makes it easier to test cases where console doesn't exist
-  let noop = function(){};
+log.warn = (...args) => logByType('warn', args);
 
-  let console = window['console'] || {
-    'log': noop,
-    'warn': noop,
-    'error': noop
-  };
-
-  if (type) {
-    // add the type to the front of the message
-    argsArray.unshift(type.toUpperCase()+':');
-  } else {
-    // default to log with no prefix
-    type = 'log';
-  }
-
-  // add to history
-  log.history.push(argsArray);
-
-  // add console prefix after adding to history
-  argsArray.unshift('VIDEOJS:');
-
-  // call appropriate log function
-  if (console[type].apply) {
-    console[type].apply(console, argsArray);
-  } else {
-    // ie8 doesn't allow error.apply, but it will just join() the array anyway
-    console[type](argsArray.join(' '));
-  }
-}
 
 export default log;

test/unit/tracks/text-track.test.js

@@ -339,20 +339,20 @@ test('tracks are parsed once vttjs is loaded', function() {
 
 test('stops processing if vttjs loading errored out', function() {
   const clock = sinon.useFakeTimers();
+  const errorSpy = sinon.spy();
   const oldVTT = window.WebVTT;
   let parserCreated = false;
   window.WebVTT = true;
 
   // use proxyquire to stub xhr module because IE8s XDomainRequest usage
   let xhrHandler;
-  let errorMsg;
   let TextTrack = proxyquire('../../../src/js/tracks/text-track.js', {
     xhr(options, fn) {
       xhrHandler  = fn;
     },
     '../utils/log.js': {
-      error(msg) {
-        errorMsg = msg;
+      'default': {
+        error: errorSpy
       }
     }
   });
@@ -376,8 +376,11 @@ test('stops processing if vttjs loading errored out', function() {
   testTech.trigger('vttjserror');
   let offSpyCall = testTech.off.getCall(0);
 
-  ok(/^vttjs failed to load, stopping trying to process/.test(errorMsg),
-     'vttjs failed to load, so, we logged an error');
+  ok(errorSpy.called, 'vttjs failed to load, so log.error was called');
+  if (errorSpy.called) {
+    ok(/^vttjs failed to load, stopping trying to process/.test(errorSpy.getCall(0).args[0]),
+       'log.error was called with the expected message');
+  }
   ok(!parserCreated, 'WebVTT is not loaded, do not try to parse yet');
   ok(offSpyCall, 'tech.off was called');
 

test/unit/utils/log.test.js

@@ -1,56 +1,75 @@
 import log from '../../../src/js/utils/log.js';
+import {logByType} from '../../../src/js/utils/log.js';
 import window from 'global/window';
 
-q.module('log');
+q.module('log', {
 
-test('should confirm logging functions work', function() {
-  let origConsole = window['console'];
-  // replace the native console for testing
-  // in ie8 console.log is apparently not a 'function' so sinon chokes on it
-  // https://github.com/cjohansen/Sinon.JS/issues/386
-  // instead we'll temporarily replace them with no-op functions
-  let console = window['console'] = {
-    log: function(){},
-    warn: function(){},
-    error: function(){}
-  };
+  beforeEach() {
 
-  // stub the global log functions
-  let logStub = sinon.stub(console, 'log');
-  let errorStub = sinon.stub(console, 'error');
-  let warnStub = sinon.stub(console, 'warn');
+    // Back up the original console.
+    this.originalConsole = window.console;
 
-  // Reset the log history
-  log.history = [];
+    // Replace the native console for testing. In IE8 `console.log` is not a
+    // 'function' so sinon chokes on it when trying to spy:
+    //   https://github.com/cjohansen/Sinon.JS/issues/386
+    //
+    // Instead we'll temporarily replace them with no-op functions
+    window.console = {
+      log: sinon.spy(),
+      warn: sinon.spy(),
+      error: sinon.spy()
+    };
+  },
+
+  afterEach() {
+
+    // Restore the native/original console.
+    window.console = this.originalConsole;
+
+    // Empty the logger's history.
+    log.history.length = 0;
+  }
+});
+
+test('logging functions should work', function() {
+
+  // Need to reset history here because there are extra messages logged
+  // when running via Karma.
+  log.history.length = 0;
 
   log('log1', 'log2');
   log.warn('warn1', 'warn2');
   log.error('error1', 'error2');
 
-  ok(logStub.called, 'log was called');
-  equal(logStub.firstCall.args[0], 'VIDEOJS:');
-  equal(logStub.firstCall.args[1], 'log1');
-  equal(logStub.firstCall.args[2], 'log2');
+  ok(window.console.log.called, 'log was called');
+  deepEqual(window.console.log.firstCall.args,
+            ['VIDEOJS:', 'log1', 'log2']);
 
-  ok(warnStub.called, 'warn was called');
-  equal(warnStub.firstCall.args[0], 'VIDEOJS:');
-  equal(warnStub.firstCall.args[1], 'WARN:');
-  equal(warnStub.firstCall.args[2], 'warn1');
-  equal(warnStub.firstCall.args[3], 'warn2');
+  ok(window.console.warn.called, 'warn was called');
+  deepEqual(window.console.warn.firstCall.args,
+            ['VIDEOJS:', 'WARN:', 'warn1', 'warn2']);
 
-  ok(errorStub.called, 'error was called');
-  equal(errorStub.firstCall.args[0], 'VIDEOJS:');
-  equal(errorStub.firstCall.args[1], 'ERROR:');
-  equal(errorStub.firstCall.args[2], 'error1');
-  equal(errorStub.firstCall.args[3], 'error2');
+  ok(window.console.error.called, 'error was called');
+  deepEqual(window.console.error.firstCall.args,
+            ['VIDEOJS:', 'ERROR:', 'error1', 'error2']);
 
   equal(log.history.length, 3, 'there should be three messages in the log history');
-
-  // tear down sinon
-  logStub.restore();
-  errorStub.restore();
-  warnStub.restore();
-
-  // restore the native console
-  window['console'] = origConsole;
+});
+
+test('in IE pre-11 (or when requested) objects and arrays are stringified', function() {
+
+  // Run a custom log call, explicitly requesting object/array stringification.
+  logByType('log', [
+    'test',
+    {foo: 'bar'},
+    [1, 2, 3],
+    0,
+    false,
+    null,
+    undefined
+  ], true);
+
+  ok(window.console.log.called, 'log was called');
+  deepEqual(window.console.log.firstCall.args,
+            ['VIDEOJS: test {"foo":"bar"} [1,2,3] 0 false null undefined']);
 });