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

Provide support of MSIE 11 msCrypto #107

Merged
merged 1 commit into from
Mar 18, 2015
Merged

Provide support of MSIE 11 msCrypto #107

merged 1 commit into from
Mar 18, 2015

Conversation

danorton-cubic-austin
Copy link
Contributor

MSIE 11 removed support for crypto, renaming their version as msCrypto.

This patch uses window.msCrypto if window.crypto is unavailable (or falls back to the uuid module's RNG).

@broofa
Copy link
Member

broofa commented Mar 17, 2015

MSDN crypto docs state the opposite, that IE11 msCrypto support is removed, replaced by [the standard] crypto property. Given that, is this diff needed?

@broofa
Copy link
Member

broofa commented Mar 17, 2015

Hmm. :)
... Hmm again

Looks like you're right. :) Ah, MSFT, how you vex me.

@@ -11,6 +11,9 @@
// returns 128-bits of randomness, since that's what's usually required
var _rng;

// Allow for MSIE11 msCrypto
var _crypto = _global.crypto || _global.msCrypto;
Copy link
Member

Choose a reason for hiding this comment

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

Can you reverse the order here? I see some reports that IE11 defines crypto, which would render the above moot. I.e., instead do this:

var _crypto = _global.msCrypto || _global.crypto;

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, but in the (very unlikely) case of both, or where someone else has defined crypto globally, do you prefer the non-standard name over the standard name?

I can say without any question that MSIE11 does not support "crypto" natively, but only supports msCrypto. (crypto was in MSIE10.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The MDN page you referenced recommends the order I proposed, as does this MS page:
https://msdn.microsoft.com/en-us/library/ie/dn265046%28v=vs.85%29.aspx

var crypto = window.crypto || window.msCrypto;

Choose a reason for hiding this comment

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

If it helps, here's a screenshot from IE 11's console on a page in edge mode.
Note the different casing - Crypto vs crypto. Weird / unexpected IMHO.

capture

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crypto != crypto
msCrypto == crypto (for our purposes, here)

broofa added a commit that referenced this pull request Mar 18, 2015
Provide support of MSIE 11 msCrypto
@broofa broofa merged commit 95a9c54 into uuidjs:master Mar 18, 2015
@broofa
Copy link
Member

broofa commented Mar 18, 2015

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants