Load without errors in IE 9#110
Conversation
…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'
lib/debug/asserts.js
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ping! Please fix, should not need to be quoted.
|
Also, please read CONTRIBUTING and follow the instructions there. You'll need to sign a license agreement and add yourself to AUTHORS & CONTRIBUTORS. Thanks! |
|
@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. |
|
Sounds good on the CLA. Just ping me when you're ready for me to take another look, and thanks for your contribution! |
|
@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! |
|
@joeyparrish We finally got the CLA taken care of, so I've pushed the new AUTHORS and CONTRIBUTORS file, so it should be GTG. |
support.js
Outdated
There was a problem hiding this comment.
Please use single quotes on these ready states. I was breaking our own style rules in the original. :-)
|
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! |
|
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 |
|
@joeyparrish Sorry about all the back and forth, but hopefully I have it all fixed now. |
lib/debug/asserts.js
Outdated
There was a problem hiding this comment.
You can drop the "type" annotation here.
|
Looks great, just one small tweak. |
|
Ok, @joeyparrish I think I have it. |
|
@joeyparrish No problem; back to you. |
|
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! |
|
No problem; thanks for sticking with all the back and forth! |
|
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
Load without errors in IE 9

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?