-
Notifications
You must be signed in to change notification settings - Fork 892
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
os() function could be improved #15
Comments
Microsoft is doing really crazy things. This adds enormous complexity for device detection. At least for this special case I "fixed" mobile-detect.js, but I won't support crazy vendors any longer. What if another vendor fakes his mobile device to be Android, iPhone, ...? Then the owners of these phones are victims to be seen as Android on one website and iPhone on another. I'd sell such a device or just throw it away. |
@davelondon Nice catch. This is one of the cases we took into account when building the I am trying to rush things up with the new detection engine which prioritize the browser/os/model detection. @hgoebl I will notify you by email when |
@serbanghita you're the man! Thanks for all your work! |
Hey guys, Have you thought of tweaking this library so it's a more exact port of the logic of the PHP version? We use this and the PHP version in tandem, and issues with the slightly different logic could make for some nasty bugs in edge cases... It seems that by design, more than one OS can be applicable to each device (WindowsPhoneOS seems to be a subset of WindowsMobileOS)... This means we have to rely on the order of the oss rules object. We shouldn't really rely on the order of a javascript object I tried to re-write my code to use the is() function instead of os(), but on closer inspection, the is() function is actually using os() under the hood. How would you like me to do a patch that re-wires the is() function to match the logic of the php is* functions - which uses matchUAAgainstKey |
Hi @davelondon, BTW I don't understand the subtle differences between WindowsPhoneOS and WindowsMobileOS, but I admit that I ignored Windows on mobile devices too much until now. |
@davelondon omg nice catch! This means that the order of keys in the JSON object is meaningless ... this is crucial for the logic of the script. I will not make this mistake for the |
already fixed in 0.4.3 |
The os() function is a bit broken... It simply cycles through the oss rules, and returns the key of the first matching rule. If it's going to do this, the rules should be carefully re-ordered. For example, this is the user agent for Windows Phone 8.1:
Mozilla/5.0 (Mobile; Windows Phone 8.1; Android 4.0; ARM; Trident/7.0; Touch; rv:11.0; IEMobile/11.0; Microsoft; Virtual) like iPhone OS 7_0_3 Mac OS X AppleWebKit/537 (KHTML, like Gecko) Mobile Safari/537
It contains the string "Android" so it matches the first "Android" rule. Here's some explanation from Microsoft: http://blogs.msdn.com/b/ie/archive/2014/07/31/the-mobile-web-should-just-work-for-everyone.aspx
Even if it didn't contain "Android", it would match the "Windows Phone [0-9.]+" part of WindowsMobileOS. This isn't ideal - I assume we should be returning WindowsPhoneOS.
I wasn't sure whether to post this here or in the Mobile_Detect library... The os() function doesn't exist in the PHP library, and perhaps this is the reason...
The text was updated successfully, but these errors were encountered: