Load without errors in IE 9 #110

spiralman · 2015-06-19T15:31:39Z

This ports our fixes back into the Shaka source.

@ustudio/dev Can somebody review this, before I open a PR back to the upstream source?

…citly checking for it

…nd is not implemented on them (IE 9 with F12 Developer Tools open)

…mplemented on it (IE 9 with F12 Developer Tools open)

… 'interactive' state, or on DOMContentLoaded. Instead, wait for 'window.onload'

joeyparrish · 2015-06-19T15:44:21Z

lib/debug/asserts.js

@@ -43,8 +43,23 @@ shaka.asserts.notImplemented = function() {};
 shaka.asserts.unreachable = function() {};


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


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.

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.

Ping! Please fix, should not need to be quoted.

joeyparrish · 2015-06-19T15:55:00Z

Also, please read CONTRIBUTING and follow the instructions there. You'll need to sign a license agreement and add yourself to AUTHORS & CONTRIBUTORS. Thanks!

spiralman · 2015-06-19T15:59:40Z

@joeyparrish Thanks for the feedback. WRT CONTRIBUTORS/AUTHORS, since I'm doing this for uStudio I need to wait for somebody else to agree to the Corporate CLA, so I left it out of the PR for now.

I actually intended to open this against our fork, so one of our other devs could review it, so you didn't have to deal with the fiddly stuff, but I guess GitHub doesn't allow that.

joeyparrish · 2015-06-19T16:11:28Z

Sounds good on the CLA. Just ping me when you're ready for me to take another look, and thanks for your contribution!

spiralman · 2015-06-19T16:28:40Z

@joeyparrish Your comments should be addressed, other than the contributor stuff. I'll update those files and ping you again when we get that sorted out on our end.

Thanks again!

spiralman · 2015-06-23T21:29:25Z

@joeyparrish We finally got the CLA taken care of, so I've pushed the new AUTHORS and CONTRIBUTORS file, so it should be GTG.

joeyparrish · 2015-06-23T21:35:11Z

support.js

+  // readyState, before document.body has been initialized, so wait
+  // for window.load
+  if (document.readyState == "loading" ||
+      document.readyState == "interactive") {


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

joeyparrish · 2015-06-25T02:43:43Z

If you can fix the quoted console['assert'] and change support.js's double quotes to single quotes, everything else looks good to me. Thanks!

joeyparrish · 2015-06-25T02:53:04Z

I spoke too soon. You'll also need to fix the errors reported by the compiler & linter. Just run ./build/all.sh to see them:

./build/all.sh
0 error(s), 0 warning(s), 98.1% typed
----- FILE : /var/lib/jenkins/jobs/Shaka Player GitHub PR Build/workspace/lib/debug/asserts.js -----
Line 46, E:0220: No docs found for member 'shaka.asserts.patchAssert_'
Line 55, E:0011: Missing semicolon after function assigned to a variable
----- FILE : /var/lib/jenkins/jobs/Shaka Player GitHub PR Build/workspace/lib/debug/log.js -----
Line 72, E:0220: No docs found for member 'shaka.log.patchConsole_'
Line 82, E:0011: Missing semicolon after function assigned to a variable
Found 4 errors, including 0 new errors, in 2 files (107 files OK).

spiralman · 2015-06-25T14:24:22Z

@joeyparrish Sorry about all the back and forth, but hopefully I have it all fixed now.

joeyparrish · 2015-06-25T23:53:10Z

lib/debug/asserts.js

@@ -43,8 +43,25 @@ shaka.asserts.notImplemented = function() {};
 shaka.asserts.unreachable = function() {};


+/** @private @type {function()} */


You can drop the "type" annotation here.

joeyparrish · 2015-06-25T23:53:49Z

Looks great, just one small tweak.

spiralman · 2015-06-26T14:14:20Z

Ok, @joeyparrish I think I have it.

joeyparrish · 2015-06-26T15:00:39Z

I'm so sorry. I know closure is new for lots of people. I meant that only @Private is needed for a private function with no arguments and no return value. We don't generally use @type for functions unless it's a callback.

spiralman · 2015-06-26T15:23:49Z

@joeyparrish No problem; back to you.

joeyparrish · 2015-06-26T15:43:40Z

Looks good to me. I'm going to see if the CI system I'm experimenting with will pick this up and test it: ok to test

If not, I'll give it a manual test run and merge later today. Thanks for your hard work on this!

spiralman · 2015-06-26T15:44:31Z

No problem; thanks for sticking with all the back and forth!

joeyparrish · 2015-06-26T16:36:28Z

CI isn't working yet, but I ran the tests manually and everything's passing. Thanks for the contribution!

Load without errors in IE 9

spiralman added 7 commits June 18, 2015 16:18

IE 9 has window.performance, but not window.performance.now, so expli…

b1baaaf

…citly checking for it

Patching console logging functions when they do not exist, or when bi…

ec623c6

…nd is not implemented on them (IE 9 with F12 Developer Tools open)

Patching console.assert when it does not exist, or when bind is not i…

2ed3478

…mplemented on it (IE 9 with F12 Developer Tools open)

Style fixes

b0a8017

Replace const with var, since const isn't supported on old IE

56df639

In IE 9, document.body does not exist yet when the document is in the…

411c738

… 'interactive' state, or on DOMContentLoaded. Instead, wait for 'window.onload'

Check for Uint8Array in support.js

c10a63d

joeyparrish reviewed Jun 19, 2015
View reviewed changes

spiralman added 2 commits June 19, 2015 11:27

Code review comments

4e14ff8

Check for Uint8Array in isBrowserSupported

7260839

Updating authors and contributors, now that we've signed the CLA

d5b4499

joeyparrish reviewed Jun 23, 2015
View reviewed changes

joeyparrish self-assigned this Jun 25, 2015

joeyparrish added the type: enhancement New feature or request label Jun 25, 2015

joeyparrish added this to the v1.4.0 milestone Jun 25, 2015

spiralman added 2 commits June 24, 2015 22:08

Code review comments

6a235a1

Adding missing docs and type annotations

5abe5b5

joeyparrish reviewed Jun 25, 2015
View reviewed changes

Removing unnecessary @type annotation

2c7196c

Removing unnecessary @type annotation

0f914cf

joeyparrish added a commit that referenced this pull request Jun 26, 2015

Merge pull request #110 from ustudio/old-ie

ccb3ed5

Load without errors in IE 9

joeyparrish merged commit ccb3ed5 into shaka-project:master Jun 26, 2015

joeyparrish mentioned this pull request Jul 1, 2015

Load without raising exceptions in old IE #87

Closed

joeyparrish added a commit that referenced this pull request Jul 6, 2015

Merge pull request #110 from ustudio/old-ie

bc02da5

Load without errors in IE 9

github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load without errors in IE 9 #110

Load without errors in IE 9 #110

spiralman commented Jun 19, 2015

joeyparrish Jun 19, 2015

spiralman Jun 19, 2015

joeyparrish Jun 23, 2015

joeyparrish commented Jun 19, 2015

spiralman commented Jun 19, 2015

joeyparrish commented Jun 19, 2015

spiralman commented Jun 19, 2015

spiralman commented Jun 23, 2015

joeyparrish Jun 23, 2015

joeyparrish commented Jun 25, 2015

joeyparrish commented Jun 25, 2015

spiralman commented Jun 25, 2015

joeyparrish Jun 25, 2015

joeyparrish commented Jun 25, 2015

spiralman commented Jun 26, 2015

joeyparrish commented Jun 26, 2015

spiralman commented Jun 26, 2015

joeyparrish commented Jun 26, 2015

spiralman commented Jun 26, 2015

joeyparrish commented Jun 26, 2015

		@@ -43,8 +43,25 @@ shaka.asserts.notImplemented = function() {};
		shaka.asserts.unreachable = function() {};


		/** @private @type {function()} */

Load without errors in IE 9 #110

Load without errors in IE 9 #110

Conversation

spiralman commented Jun 19, 2015

joeyparrish Jun 19, 2015

Choose a reason for hiding this comment

spiralman Jun 19, 2015

Choose a reason for hiding this comment

joeyparrish Jun 23, 2015

Choose a reason for hiding this comment

joeyparrish commented Jun 19, 2015

spiralman commented Jun 19, 2015

joeyparrish commented Jun 19, 2015

spiralman commented Jun 19, 2015

spiralman commented Jun 23, 2015

joeyparrish Jun 23, 2015

Choose a reason for hiding this comment

joeyparrish commented Jun 25, 2015

joeyparrish commented Jun 25, 2015

spiralman commented Jun 25, 2015

joeyparrish Jun 25, 2015

Choose a reason for hiding this comment

joeyparrish commented Jun 25, 2015

spiralman commented Jun 26, 2015

joeyparrish commented Jun 26, 2015

spiralman commented Jun 26, 2015

joeyparrish commented Jun 26, 2015

spiralman commented Jun 26, 2015

joeyparrish commented Jun 26, 2015