-
Notifications
You must be signed in to change notification settings - Fork 507
Improves version detection for Mobile Safari #8027
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
base: master
Are you sure you want to change the base?
Conversation
| if (\in_array($this->osName, ['iOS', 'iPadOS']) && '' !== $this->osVersion && 'Mobile Safari' === $name) { | ||
| $version = $this->osVersion; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just this condition will be enough for us?
// Mobile Safari version is always the iOS / iPadOS version
// See https://developer.apple.com/documentation/safari-release-notes
if ('Mobile Safari' === $name && '' === $version) {
\preg_match(
'~iP(?:hone|ad) ?OS ([0-9]{1,2})[_.]([0-9]{1,2})(?:[_.]([0-9]{1,2}))?~i',
$this->userAgent,
$osMatch
);
if ($osMatch) {
$version = \implode('.', \array_filter([$osMatch[1], $osMatch[2], $osMatch[3] ?? null]));
}
}
Then our browser parser will not depend on OS parser.
This will also work correctly when using BrowserParser alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just this condition will be enough for us?
Then our browser parser will not depend on OS parser. This will also work correctly when using BrowserParser alone.
It will not work because there are user agents like:
MobileSafari/9537.53 CFNetwork/672.1.13 Darwin/13.1.0
Another way I tried to do this is was to change the browser version with os version in DeviceDetector.php, with only a few lines, but then I encountered the failed tests, which use only the BrowserParser.
I'm happy with any other suggestions, if we can do this, and tests pass.
#7981 (comment)
Mobile Safari version is always the iOS / iPadOS version
https://developer.apple.com/documentation/safari-release-notes
This should also close #7979.