@sgiehl opened this Pull Request on February 24th 2022 Member

Description:

this requires https://github.com/matomo-org/device-detector/pull/6989 to be finished and released

fixes #16125

Review

@sgiehl commented on February 24th 2022 Member

build js

@justinvelluppillai commented on March 21st 2022 Contributor

How are we looking here @sgiehl? Anything we need to do to push this forward?

@sgiehl commented on March 21st 2022 Member

@justinvelluppillai Had to rewrite the JS a bit. As the former version didn't work in all cases. Once tests are fixed and added, this should be ready for a first review.

@sgiehl commented on April 14th 2022 Member

Had some more thoughts on that one today. The implementation is kind of finished, but some more tests would be needed.
We also need to consider to make it possible to set the client hints in Matomo PHP Tracker, so we are also able to add some tests where those data is provided.
The general implementation should be ready for a first review.

@github-actions[bot] commented on April 29th 2022 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@MatomoForumNotifications commented on May 10th 2022

This pull request has been mentioned on Matomo forums. There might be relevant details there:

https://forum.matomo.org/t/windows-11-recorded-as-windows-10/45882/2

@justinvelluppillai commented on May 17th 2022 Contributor

@bx80 although still in draft this is needing a more detailed review. Are you able to take ownership of reviewing this PR and take a look this week?

@bx80 commented on May 20th 2022 Contributor

build js

@github-actions[bot] commented on May 27th 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions[bot] commented on June 4th 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@sgiehl commented on June 9th 2022 Member

One minor thing I noticed is that the stored browser version is a lot more detailed when obtained via client hints. On 4.x-dev I get config_browser_name = Chrome, config_browser_version = 101.0 whereas with client hints the same browser is recorded as config_browser_name = Chrome, config_browser_version = 101.0.4951.54. This will reduce the usefulness of the browser version metric where there may be many 101.0.xxxx.xx rows. Would it perhaps be worth attempting to truncate the version string to the major version number for known browsers?

Good catch. The device detector is actually also able to detect such detailed versions, but returns the details requested. For Matomo we are using the default, which automatically truncates to the minor version number. So guess we should do that for client hints as well.

@sgiehl commented on June 21st 2022 Member

I've update the PR so it now also includes some changes on related libs & plugins.
After final review has been done the following steps need to be performed before merging:

@justinvelluppillai commented on June 22nd 2022 Contributor

@bx80 it'd be great if you could give this a final review after the latest changes

@sgiehl commented on June 29th 2022 Member

@bx80 did you also check the PRs I have mentioned in the previous comment? Would be good to have them reviewed as well.

@bx80 commented on June 30th 2022 Contributor

@sgiehl I've now reviewed the additional PRs, can't see any issues other than a test needing an update :+1:

Powered by GitHub Issue Mirror