Improves version detection for Opera and Opera Mobile and improves browser version detection for client hints #7065
Conversation
liviuconcioiu
changed the title
| if (!empty($browserFromUserAgent['version']) | ||
| && 0 === \strpos($browserFromUserAgent['version'], $version) | ||
| && \version_compare($version, $browserFromUserAgent['version'], '<') | ||
| ) { | ||
| $version = $browserFromUserAgent['version']; | ||
| } |
There was a problem hiding this comment.
This shouldn't be moved outside of the if condition. Let's assume the client hints report a Firefox 16, but the useragent is spoofed by some tool and contains Safari 20.18. We would then end up using Firefox 20.18, which is wrong.
There was a problem hiding this comment.
And, how can we detect the full version then? Some times, client hints don't provide Sec-CH-UA-Full-Version
There was a problem hiding this comment.
In most cases the browser reported in client hints and in user agent should match. In that case the version from user agent will be used. If not I'm not sure if we should simply use the version from useragent.
There was a problem hiding this comment.
it turns out to be interesting here, opera prints all versions in clienthints
04/04/2022
last version Opera Desktop Linux 85
last version Opera Mobile 68.2
| Opera on Windows | Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.60 Safari/537.36 OPR/85.0.4341.18Mozilla/5.0 (Windows NT 10.0; WOW64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.60 Safari/537.36 OPR/85.0.4341.18 |
|---|---|
| Opera on Macos | Mozilla/5.0 (Macintosh; Intel Mac OS X 12_3_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.60 Safari/537.36 OPR/85.0.4341.18 |
| Opera on Linux | Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.60 Safari/537.36 OPR/85.0.4341.18 |
| Opera on Android | Mozilla/5.0 (Linux; Android 10; VOG-L29) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.58 Mobile Safari/537.36 OPR/63.3.3216.58675 |
| -- | -- |
| Opera on Android | Mozilla/5.0 (Linux; Android 10; SM-G970F) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.58 Mobile Safari/537.36 OPR/63.3.3216.58675Mozilla/5.0 (Linux; Android 10; SM-N975F) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.58 Mobile Safari/537.36 OPR/63.3.3216.58675 |
if you look at this list, they just printed versions for all OS
Sec-CH-UA: '"Chromium";v="98", "Opera";v="82", " Not A;Brand";v="99", "OperaMobile";v="68"'
Sec-CH-UA-Platform: 'Android'
the current tests I am satisfied with the result.
But in the future it will be necessary to come up with something else when useragent is gone.
as an option:
if Mobile OS + opera family get clienthint OperaMobile=68
if Desktop OS + opera family get Opera=82
Parser/Client/Browser.php
Outdated
|
|
||
| // If client hints report Opera or Opera Mobile, we use the version from useragent | ||
| if (!empty($browserFromUserAgent['version']) | ||
| && ('OP' === $short || 'OM' === $short) |
There was a problem hiding this comment.
Is it known that Opera always reports an incorrect version in the client hints? 🤔
There was a problem hiding this comment.
From the data I have, everything is incorrect, just for those two browser.
There was a problem hiding this comment.
Hm. I'm not sure how we should handle that. If the browsers might freeze the useragent at some point I'm not sure if the version in useragent will be even updated with new releases or if the number can only be fetched through client hints 🤔
There was a problem hiding this comment.
Should I close this PR then?
There was a problem hiding this comment.
@sanchezzzhak what do you think about that?
There was a problem hiding this comment.
The current changes give an accurate result.
but if there is no user agent in the future, this condition will need to be rewritten.
liviuconcioiu
changed the title
|
I will try to deal with this PR in the near 1 week (I think it's tedious to update it to the current state every time) |
|
Accepted because the browser versions presented in the tests meet the expectations: |
No description provided.