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

Browserslist entries that have version ranges don't match valid browsers #41

Open
litonico opened this issue May 30, 2019 · 4 comments
Open

Comments

@litonico
Copy link

litonico commented May 30, 2019

browserslist contains entries with version ranges. For instance, browserslist("ios_saf >= 9") includes the entry 'ios_saf 9.0-9.2'.
For that browserslist, browserslist-useragent matches only ios_saf 9.0. User agents with iOS Safari versions of 9.1 or 9.2 are never matched.

Steps to Repro:

const { matchesUA, resolveUserAgent } = require('browserslist-useragent')
const browserslist = require('browserslist')
ua = 'Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1 (KHTML, like Gecko) CriOS/74.0.3729.169 Mobile/13B143 Safari/601.1.46'

console.log(resolveUserAgent(ua))
// > { family: 'iOS', version: '9.1.0' } // This is correct

console.log(matchesUA(ua, { browsers: ["ios_saf >= 9"] }))
// > false // This should be `true`

Expected behavior:
matchesUA('Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1 (KHTML, like Gecko) CriOS/74.0.3729.169 Mobile/13B143 Safari/601.1.46', { browsers: ["ios_saf >= 9"] }))
returns true

Actual behavior:
matchesUA('Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1 (KHTML, like Gecko) CriOS/74.0.3729.169 Mobile/13B143 Safari/601.1.46', { browsers: ["ios_saf >= 9"] }))
returns false

What's happening:

normalizedVersion = browserVersion.split('-')[0]

Splitting the string 'ios_saf 9.0-9.2' and taking the first array element means the only matched version is 9.0. 9.1 and 9.2 are incorrectly not matched.

@onoshkodaniil
Copy link

@litonico did you try allowHigherVersions option?

@litonico
Copy link
Author

litonico commented Jul 10, 2019

@onoshkodaniil
That option makes the matchesUA query correct in some cases, but that's an unrelated feature — this is still a bug. Similarly, ignoreMinor mitigates this issue, which is what we're doing in production right now. However, using that means that we are likely serving unsupported content to some users!

@pastelsky
Copy link
Collaborator

TBH, I'm not sure why I did the split like that. It makes little sense now that I think of it. I just released 3.0.1 which should now respect version ranges instead of ignoring them (as long as the ranges are minor ranges, which seems to be true with the current output of browserslist).

Note, however, this issue is still not completely fixed due to a bug in
browserslist – browserslist/browserslist#402

@pastelsky pastelsky reopened this Jul 31, 2019
@mrdimidium
Copy link

mrdimidium commented Oct 2, 2019

Hi. I still see strange behavior in 3.0.2 versions. For example:

const { matchesUA } = require("browserslist-useragent");

matchesUA("Chrome/77", { browsers: ["defaults"] }); // => return false
matchesUA("Chrome/77.0.3865.75", { browsers: ["defaults"] }); // => but this return true

cc @pastelsky

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

No branches or pull requests

4 participants