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

Cross browser prefix detection #173

Merged
merged 2 commits into from
Aug 22, 2017
Merged

Cross browser prefix detection #173

merged 2 commits into from
Aug 22, 2017

Conversation

joao-arvins
Copy link
Contributor

Updated logic to set prefix so that it works in all browsers.

Updated logic to set prefix so that it works in all browsers.
Merged all matches into a single regex expression
@joao-arvins
Copy link
Contributor Author

@csuwildcat all the checks are now collapsed into a single regexp. Can you have a look when you have some time please?

Thanks

@joao-arvins
Copy link
Contributor Author

Hi @csuwildcat . Did you had the opportunity to have a look at this pull request?

Cheers

@csuwildcat
Copy link
Member

I did - I had that one last question before merging. Can you take a look and answer for me?

@joao-arvins
Copy link
Contributor Author

Not sure about the question you're talking about.
Is it the one about collapse all the checks into one single regexp? If so I've already updated it some weeks ago. Check the latest commit.

If not can you point me to the right direction?

Thanks

src/core.js Outdated
@@ -33,7 +33,7 @@
*/
prefix = (function () {
var keys = Object.keys(Object.getPrototypeOf(window)).concat(Object.keys(window)).join();
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for using window,getPrototypeOf() to get the keys from the window prototype? Why doesn't Object.keys(window) alone suffice?

@csuwildcat csuwildcat merged commit f405189 into x-tag:master Aug 22, 2017
@toreholmberg
Copy link

Did this make it into the build?

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.

3 participants