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

Improves version detection for Opera and Opera Mobile and improves browser version detection for client hints #7065

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

liviuconcioiu
Copy link
Collaborator

No description provided.

@liviuconcioiu liviuconcioiu changed the title Adds detection for Wave Browser and improves version detection for Opera and Opera Mobile Adds detection for Wave Browser, improves version detection for Opera and Opera Mobile and improves browser version detection for client hints Mar 25, 2022
Comment on lines +671 to +676
if (!empty($browserFromUserAgent['version'])
&& 0 === \strpos($browserFromUserAgent['version'], $version)
&& \version_compare($version, $browserFromUserAgent['version'], '<')
) {
$version = $browserFromUserAgent['version'];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, how can we detect the full version then? Some times, client hints don't provide Sec-CH-UA-Full-Version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// If client hints report Opera or Opera Mobile, we use the version from useragent
if (!empty($browserFromUserAgent['version'])
&& ('OP' === $short || 'OM' === $short)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it known that Opera always reports an incorrect version in the client hints? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the data I have, everything is incorrect, just for those two browser.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I close this PR then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sanchezzzhak what do you think about that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 liviuconcioiu changed the title Adds detection for Wave Browser, improves version detection for Opera and Opera Mobile and improves browser version detection for client hints Improves version detection for Opera and Opera Mobile and improves browser version detection for client hints May 19, 2022
@sanchezzzhak
Copy link
Collaborator

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)

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.

None yet

3 participants