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
add disable finger print options #18599
Conversation
add disable finger print
update eslint and piwijs
…atomo into m-18448-disable-fingerprint
update name
@tsteur sorry to bother, just checking if I am on the right track. |
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.
@peterhashair left a few comments, overall goes in the right direction
update naming and functions
add disableBrowserFeatureDetection to apply first
@peterhashair why are you adding those eslint rules? |
@justinvelluppillai |
remove eslint rules
reset
swap apply event place
apply ignores
build js |
@peterhashair be good to get this one ready for next review and request reviewer if it is being ignored. Also looking at the FAQ I think maybe it is better to add the information to this page which describes how the config_id works more generally also https://matomo.org/faq/general/how-is-the-visitor-config_id-processed/ (or at least link to this page and the page on fingerprinting). |
@justinvelluppillai sure, will do. |
move disableBrowserFeatureDetection up
build js |
move function below
update format
update some naming
Co-authored-by: Justin Velluppillai <justin@innocraft.com>
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.
Working for me, looking good also. @tsteur did you want to have a look?
@justinvelluppillai I haven't fully tested it but I think we could merge for now so it's in the release. Then add some follow up work like add also an |
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.
Works for me. It would probably be a good idea to have a tracker test to make sure screen resolution and other fingerprint data are not recorded when this option is enabled to prevent any future regression.
This is looking good thanks @peterhashair, still good to publish your FAQ and update the other mentioned doc, after that once these tests pass this is good to merge and will probably be the last merge before the next RC. |
@justinvelluppillai updated the FAQ, just reference a link at the bottom. publish it now. |
Description:
Fixes: #18448
add disable finger print
FAQ Preview: https://matomo.org/faq/how-do-i-disable-browser-feature-detection-completely/
Review