Looks good so far. We could maybe add a UI test to make sure this will keep working. @flamisz it's possible to manipulate the useragent used in UI tests (see https://github.com/matomo-org/matomo/blob/4.x-dev/tests/UI/specs/OptOutForm_spec.js)
Do I understand it correctly that this runs the users user-agent through device-detector on any single request?
I can imagine that there are quite a few slow servers out there (or ones without regex-caching) where this could add more than just a few milliseconds to every request.
Maybe just adding it to the login should be enough (unless that's what the code already does).
BTW only running it on login could work for regular requests but for widgets it would always need to run (eg when
Piwik::getModule()===Widgetize), it would also need to always run for anonymous users, and for API module/plugin we wouldn't want it to run (for performance etc since it wouldn't be "rendered" in the browser)
@tsteur I modified the logic: only checks the browser if the user is
anonymous and the module is not
API. This way even the widgets won't be checked if you are in the same browser, and of course before you log in, it will be checked.
Hey @tsteur I modified the PR according to your comments. Please view it again when you have some time.
@flamisz overall looks good from my perspective 👍 but didn't test it
@tsteur I did my best to test on a couple of browsers and a couple of use cases with BrowserStack.
And of course, we have some test cases as well.
👍 feel free to merge if the tests pass (didn't check)