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

Use Feature Detection instead of Browser Sniffing #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

willsp
Copy link

@willsp willsp commented Sep 18, 2014

While the previous implementation used feature detection, it was using
it wrong. It was, essentially using feature detection as a more advanced
browser sniffer.

  • Updated this to detect the features actually being used to measure
    zoom.
  • Moved the functions to the same place as the checks, to make it
    easier to keep the 2 connected.
  • Made some adjustments so it passes jshint.
  • This fixes a bug with iOS8 Safari.

While the previous implementation used feature detection, it was using
it wrong. It was, essentially using feature detection as a more advanced
browser sniffer.

* Updated this to detect the features actually being used to measure
  zoom.
* Moved the functions to the same place as the checks, to make it
  easier to keep the 2 connected.
* Made some adjustments so it passes jshint.
@bradvogel
Copy link

This unfortunately doesn't work within iframes.

@willsp
Copy link
Author

willsp commented Nov 10, 2015

Ah, that's not cool... Not really a use case for us. Did the original work with iframes?

If you have any suggestions (or better yet, code), let me know and I'll see if I can make it work...

@bradvogel
Copy link

Never tried the original. Seems to be a couple of years out date. Lack of iframe support shouldn't prevent you from merging though - this PR is still very useful! I'll look into adding support for iframes.

@ericsaboia
Copy link

any updates on this?

@willsp
Copy link
Author

willsp commented Apr 21, 2016

@ericsaboia Nope... Sorry. I'm no longer using it, so have little reason to get it working for iframes. If you fix it and submit a pull request to my fork, I'll merge it.

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