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.

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 raising exceptions in old IE #87

Closed
spiralman opened this issue Jun 2, 2015 · 5 comments
Closed

Load without raising exceptions in old IE #87

spiralman opened this issue Jun 2, 2015 · 5 comments
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@spiralman
Copy link
Contributor

Currently, the shaka player script fails to load on IE 9 and earlier, due to several accesses to non-existent functions/APIs that run at load time. I.e. if I build a player, with shaka, as a single concatenated JS file, the player will fail to load entirely on IE 9 and earlier.

This causes two main problems:

  1. It means that, to use Shaka from a player that needs to support old versions of IE, you need to dynamically load the script file only when playing back using MPEG-DASH, which adds a lot of a complexity to the player.
  2. It also means that the player can't use functions like shaka.player.Player.isBrowserSupported to safely check for DASH support, because it can't load shaka at all until it knows the browser is supported.

I would be interested in opening up a PR to fix these issues, but I would like to know if there is a desire for the project to support this use-case.

Here is a random selection of the first few issues I've run into while trying to get the code to load in IE 9:

ebml_parser.js:14:

shaka.util.EbmlParser.DYNAMIC_SIZES = [
  new Uint8Array([0xff]), 
  // ...
]

Uint8Array does not exist, but is being invoked in the root of the script.

patchedmediakeys_v01b.js:892:

shaka.polyfill.PatchedMediaKeys.v01b.MediaKeyStatusMap.KEY_ID_ =
  shaka.util.Uint8ArrayUtils.fromString('FAKE_KEY_ID');

Uint8ArrayUtils.fromString calls new Uint8Array internally.

debug/timer.js:117:

shaka.timer.now_ = window.performance ?
                       window.performance.now.bind(window.performance) :
                       Date.now;

window.performance does exist in IE 9, but window.performance.now does not.

debug/log.js contains several calls to console.log|error|warn|etc.bind, but console.log and its ilk are not actual function objects in IE 9, and thus do not have a bind, nor is it patchable via, e.x., es5shim.

@joeyparrish joeyparrish added type: enhancement New feature or request flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this labels Jun 2, 2015
@joeyparrish
Copy link
Member

I would be very happy to accept a pull request to resolve these issues.

So far we've found Uint8Array, window.performance, and console issues.

The performance.now problem seems easy enough to fix, for example, by checking for (window.performance && window.performance.now).

Do you have any ideas for dealing with load-time uses of Uint8Array?

Once these things are not causing load-time exceptions, we should add them to isBrowserSupported and the browser support test page (support.html/js).

@spiralman
Copy link
Contributor Author

For Uint8Array, my original plan was to remove all the static uses of them, and make them dynamic on initialization time. However, after digging in a little more, I think it might be easier to "poly fill" them as no op functions.

Since the browsers without those types won't work anyway, it's just a matter of getting past load time exceptions.

If I do go down that path, would it be best to add them as a new poly fill, or just statically in a module that uses them? It seems adding them as a poly fill module would break a pattern, since those are all installed by the calling code, rather than loaded implicitly as these would have to be.

joeyparrish added a commit that referenced this issue Jun 2, 2015
This is inteded to fix a couple of the load-time exceptions seen
when loading the library on IE9.  Eliminating load-time exceptions
will make it possible to use isBrowserSupported() as intended, to
avoid use of the library on older browsers.

Issue #87

Change-Id: I993d4f955e80a7401bea182ae90df43a8a022ca2
@joeyparrish
Copy link
Member

Yes, you are right. If we go with a nop-polyfill, the trouble will be that the polyfill must be loaded and installed before loading the library.

It seems to me that we assumed that we could construct Uint8Arrays at load-time. Since that assumption is false, I believe we should just not use them at load-time.

I moved the construction of PatchedMediaKeys.v01b.MediaKeyStatusMap.KEY_ID_ to v01b.install(). It seems that the closure compiler will still allow you to do this for a const type, so long as it is only set once.

I handled EbmlParser.DYNAMIC_SIZES similarly in EbmlParser's constructor, only there I'm checking to see if it's already been constructed, since EbmlParser's constructor could easily be called many times.

I don't have IE9 to test with, but please take 1e400f5 as an example and I'll let you take it from here. If you run into anything where the fix is not obvious or affects design in some larger way, we can discuss here. If you haven't read CONTRIBUTING.md and signed the contribution agreement, please do that before submitting a PR. Thanks so much for looking into this!

@spiralman
Copy link
Contributor Author

Thanks for getting started on this. I have a few things going on this week, but I'll hope to get started on a PR tomorrow. I'll be sure to check the contributing document, and sign the contribution agreement.

@joeyparrish joeyparrish added this to the v1.4.0 milestone Jul 1, 2015
@joeyparrish
Copy link
Member

Resolved by #110. Thanks!

joeyparrish added a commit that referenced this issue Jul 6, 2015
This is inteded to fix a couple of the load-time exceptions seen
when loading the library on IE9.  Eliminating load-time exceptions
will make it possible to use isBrowserSupported() as intended, to
avoid use of the library on older browsers.

Issue #87

Change-Id: I993d4f955e80a7401bea182ae90df43a8a022ca2
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants