Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Make Android Browser 4 core #911

Merged
merged 4 commits into from
Apr 24, 2017
Merged

Make Android Browser 4 core #911

merged 4 commits into from
Apr 24, 2017

Conversation

leggsimon
Copy link
Contributor

No description provided.

try {
getRandomValuesAvailable = 'getRandomValues' in window.crypto
} catch (e) {
getRandomValuesAvailable = 'getRandomValues' in window.msCrypto
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also be in a try/catch to prevent errors on non-IE browsers which don't support .crypto?

Alternatively avoid the whole nasty try catch with:

var crypto = window.crypto || window.msCrypto;
if (crypto) {
   getRandomValuesAvailable = 'getRandomValues' in crypto;
}

maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

(yes)

Copy link
Contributor Author

@leggsimon leggsimon Apr 10, 2017

Choose a reason for hiding this comment

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

God yes, that is much nicer 🤦‍♂️ And a good point about non-IE browsers

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.453% when pulling 20bbf47 on make-android-browser-4-core into d75add5 on master.

@leggsimon
Copy link
Contributor Author

leggsimon commented Apr 10, 2017

It doesn't work 🙈

@rowanbeentje
Copy link
Contributor

On 4.x? Ouch 🙈 I did only test it on a Nexus and iPhone...

@leggsimon
Copy link
Contributor Author

Doesn't seem to, no

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.453% when pulling 692b5e9 on make-android-browser-4-core into d75add5 on master.

getRandomValuesAvailable = 'getRandomValues' in cryptoApi;
var supportsBlobConstructor;
try {
new Blob;
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it also knocks out UCBrowser (10% global usage???). Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YAY! don't have to worry!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you've excluded a Sauce browser of some variety though ;)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.453% when pulling 692b5e9 on make-android-browser-4-core into d75add5 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.453% when pulling 692b5e9 on make-android-browser-4-core into d75add5 on master.

if (window.cutsTheMustard) {
alert('cuts the mustard')
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for the alert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I've noticed that 🙈

@leggsimon leggsimon merged commit edc0613 into master Apr 24, 2017
@leggsimon leggsimon deleted the make-android-browser-4-core branch April 24, 2017 10:58
wheresrhys added a commit that referenced this pull request Apr 24, 2017
… rhys/polyfill-config

* 'master' of https://github.com/Financial-Times/n-ui:
  Make Android Browser 4 core (#911)
  Rhys/decouple asset hashing (#920)
wheresrhys added a commit that referenced this pull request Apr 24, 2017
… rhys/brotli-again

* 'master' of https://github.com/Financial-Times/n-ui:
  Make Android Browser 4 core (#911)
  Rhys/decouple asset hashing (#920)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants