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 7 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
16 changes: 15 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,23 @@ shaka.asserts.notImplemented = function() {};
shaka.asserts.unreachable = function() {};


shaka.asserts.patchAssert_ = function() {
var assert = console['assert'];
Copy link
Member

Choose a reason for hiding this comment

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

I think this doesn't need to be quoted. Dot notation should work fine, since closure will not rename browser-built-ins and thingThatExists.nonExistentMember will be undefined, but not cause an exception. Please correct me if I missed something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. For some reason, I thought IE had tripped up when I didn't quote it, but I can't reproduce that behavior, so there must have been something else wrong.

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 fix, should not need to be quoted.


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.

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 +69,3 @@ if (shaka.asserts.ENABLE_ASSERTS) {
shaka.asserts.unreachable =
console.assert.bind(console, 0 == 1, 'Unreachable reached.');
}

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

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.

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) {
/**
Expand Down Expand Up @@ -118,4 +136,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_ = {};

42 changes: 22 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,9 @@ if (async.length) {
}

function onLoaded(fn) {
if (document.readyState == "loading") {
document.addEventListener('DOMContentLoaded', fn);
if (document.readyState == "loading" ||
document.readyState == "interactive") {
Copy link
Member

Choose a reason for hiding this comment

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

Please use single quotes on these ready states. I was breaking our own style rules in the original. :-)

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