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.

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

Load without errors in IE 9 #110

Merged
merged 14 commits into from
Jun 26, 2015
Merged

Load without errors in IE 9 #110

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b1baaaf
IE 9 has window.performance, but not window.performance.now, so expli…
spiralman Jun 18, 2015
ec623c6
Patching console logging functions when they do not exist, or when bi…
spiralman Jun 18, 2015
2ed3478
Patching console.assert when it does not exist, or when bind is not i…
spiralman Jun 18, 2015
b0a8017
Style fixes
spiralman Jun 19, 2015
56df639
Replace const with var, since const isn't supported on old IE
spiralman Jun 19, 2015
411c738
In IE 9, document.body does not exist yet when the document is in the…
spiralman Jun 19, 2015
c10a63d
Check for Uint8Array in support.js
spiralman Jun 19, 2015
4e14ff8
Code review comments
spiralman Jun 19, 2015
7260839
Check for Uint8Array in isBrowserSupported
spiralman Jun 19, 2015
d5b4499
Updating authors and contributors, now that we've signed the CLA
spiralman Jun 23, 2015
6a235a1
Code review comments
spiralman Jun 25, 2015
5abe5b5
Adding missing docs and type annotations
spiralman Jun 25, 2015
2c7196c
Removing unnecessary @type annotation
spiralman Jun 26, 2015
0f914cf
Removing unnecessary @type annotation
spiralman Jun 26, 2015
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
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ Google Inc. <*@google.com>
Jason Palmer <jason@jason-palmer.com>
Oskar Arvidsson <oskar@irock.se>
Sanborn Hilland <sanbornh@rogers.com>
uStudio Inc. <*@ustudio.com>
1 change: 1 addition & 0 deletions CONTRIBUTORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Joey Parrish <joeyparrish@google.com>
Natalie Harris <natalieharris@google.com>
Oskar Arvidsson <oskar@irock.se>
Sanborn Hilland <sanbornh@rogers.com>
Thomas Stephens <thomas@ustudio.com>
Timothy Drews <tdrews@google.com>
Vasanth Polipelli <vasanthap@google.com>
Vignesh Venkatasubramanian <vigneshv@google.com>
Expand Down
18 changes: 17 additions & 1 deletion lib/debug/asserts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,25 @@ shaka.asserts.notImplemented = function() {};
shaka.asserts.unreachable = function() {};


/** @private @type {function()} */
Copy link
Member

Choose a reason for hiding this comment

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

You can drop the "type" annotation here.

shaka.asserts.patchAssert_ = function() {
var assert = console.assert;

if (!assert) {
console.assert = function() {};
} else if (!assert.bind) {
Copy link
Member

Choose a reason for hiding this comment

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

A comment here about what platforms/browsers do not have console.assert.bind would be helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Ping! Please add a comment here about who does not have console.assert.bind and why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is one here; I added it inside the if, but I can try to fit it on the end of the line, if that's more in-line with your style.

// IE 9 does not provide a .bind for the built-in console functions.
console.assert = function() {
assert.apply(console, arguments);
};
}
};


// Install assert functions.
if (shaka.asserts.ENABLE_ASSERTS) {
shaka.asserts.patchAssert_();

shaka.asserts.assert =
console.assert.bind(console);

Expand All @@ -54,4 +71,3 @@ if (shaka.asserts.ENABLE_ASSERTS) {
shaka.asserts.unreachable =
console.assert.bind(console, 0 == 1, 'Unreachable reached.');
}

25 changes: 24 additions & 1 deletion lib/debug/log.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,30 @@ shaka.log.v1 = function() {};
shaka.log.v2 = function() {};


/**
* @private
* @param {string} logName
*/
shaka.log.patchConsole_ = function(logName) {
var nop = function() {};
var logFunction = console[logName];

if (!logFunction) {
console[logName] = nop;
} else if (!logFunction.bind) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, a comment about why/where bind might be missing would be useful.

Copy link
Member

Choose a reason for hiding this comment

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

Ping! A comment about why/where bind is missing, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here; should I move it up?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, looks good.

// IE 9 does not provide a .bind for the built-in logging functions.
console[logName] = function() {
logFunction.apply(console, arguments);
};
}
};

shaka.log.patchConsole_('error');
shaka.log.patchConsole_('warn');
shaka.log.patchConsole_('info');
shaka.log.patchConsole_('log');
shaka.log.patchConsole_('debug');

if (!COMPILED) {
/**
* Change the log level. Useful for debugging in uncompiled mode.
Expand Down Expand Up @@ -118,4 +142,3 @@ if (shaka.log.MAX_LOG_LEVEL >= shaka.log.Level.V1) {
if (shaka.log.MAX_LOG_LEVEL >= shaka.log.Level.V2) {
shaka.log.v2 = console.debug.bind(console);
}

3 changes: 1 addition & 2 deletions lib/debug/timer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,10 @@ shaka.timer.get = function(name) {


/** @private {function():number} */
shaka.timer.now_ = window.performance ?
shaka.timer.now_ = window.performance && window.performance.now ?
window.performance.now.bind(window.performance) :
Date.now;


/** @private {!Object.<string, {begin: number, end: number}>} */
shaka.timer.timers_ = {};

5 changes: 3 additions & 2 deletions lib/player/player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ shaka.player.Player.isBrowserSupported = function() {
!!document.exitFullscreen &&
'fullscreenElement' in document &&
// Node.children is used by mpd_parser.js, and body is a Node instance.
!!document.body.children;
!!document.body.children &&
// Uint8Array is used frequently for parsing binary data
!!window.Uint8Array;
};


Expand Down Expand Up @@ -956,4 +958,3 @@ shaka.player.Player.MEDIA_ERROR_MAP_ = {
4: // MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED
'The browser does not support the media content.'
};

45 changes: 25 additions & 20 deletions support.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,34 @@
*/

// Status values for the report entries.
const kGood = 0;
const kInfo = 1;
const kBad = 2;

const vp8Type = 'video/webm; codecs="vp8"';
const vp9Type = 'video/webm; codecs="vp9"';
const mp4Type = 'video/mp4; codecs="avc1.42E01E"';
const tsType = 'video/mp2t; codecs="avc1.42E01E"';

const clearKeyId = 'org.w3.clearkey';
const widevineId = 'com.widevine.alpha';
const playReadyId = 'com.microsoft.playready';
const adobeAccessId = 'com.adobe.access';
const fairPlayId = 'com.apple.fairplay';

const classPrefixes = [
var kGood = 0;
var kInfo = 1;
var kBad = 2;

var vp8Type = 'video/webm; codecs="vp8"';
var vp9Type = 'video/webm; codecs="vp9"';
var mp4Type = 'video/mp4; codecs="avc1.42E01E"';
var tsType = 'video/mp2t; codecs="avc1.42E01E"';

var clearKeyId = 'org.w3.clearkey';
var widevineId = 'com.widevine.alpha';
var playReadyId = 'com.microsoft.playready';
var adobeAccessId = 'com.adobe.access';
var fairPlayId = 'com.apple.fairplay';

var classPrefixes = [
'WebKit',
'MS',
'Moz'
];

const propertyPrefixes = [
var propertyPrefixes = [
'webkit',
'ms',
'moz'
];

const keySystemPrefixes = [
var keySystemPrefixes = [
'webkit-'
];

Expand Down Expand Up @@ -230,6 +230,7 @@ function testForKeySystem(ks, required) {
testForClass(window, 'HTMLMediaElement', true);
testForClass(window, 'MediaSource', true);
testForClass(window, 'Promise', true);
testForClass(window, 'Uint8Array', true);
Copy link
Member

Choose a reason for hiding this comment

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

Would you also add Uint8Array to shaka.player.Player.isBrowserSupported()? Ideally, that and this should agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.


// Optional:
testForClass(window, 'VTTCue', false);
Expand Down Expand Up @@ -303,8 +304,12 @@ if (async.length) {
}

function onLoaded(fn) {
if (document.readyState == "loading") {
document.addEventListener('DOMContentLoaded', fn);
// IE 9 fires DOMContentLoaded, and enters the "interactive"
// readyState, before document.body has been initialized, so wait
// for window.load
if (document.readyState == 'loading' ||
document.readyState == 'interactive') {
window.addEventListener('load', fn);
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing IE9 doesn't have DOMContentLoaded either? A comment would be nice to remind me not to change it back when I stumble across this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but it fires when the document enters the 'interactive' state, at which point document.body is still null.

I think this isn't ideal on a real page, since it waits for images, etc, to load, but since it's just the support page (and has no external resources) it seems best to keep it simple.

} else {
fn();
}
Expand Down