add file:// protocal exclude option in tracker #19835
Conversation
add file protocal option in js
|
Not too sure if we can write a test for that feature, to change window.location.protocal maybe a problem. |
remove function tests
|
@sgiehl just checking, if that is the right solution. |
|
@peterhashair what should your PR provide? Currently it adds a config option, that can be changed. But the option is not having any effect, so it's kind of useless 🤔 |
|
@sgiehl I thought this would escape the tracking. if (!configFileTracking && windowAlias.location.protocol === 'file:') {
configDoNotTrack = true;
} |
Isn't that variable responsible for something completely different? Didn't have look right now, but iirc that should be the config flag responsible for checking "do-not-track" headers in the browser. |
return empty
peterhashair
added
c: Documentation
move file protocol check after do not track
There was a problem hiding this comment.
Just so we don't forget: Once this feature is finished, we need to mention it as a breaking change in the CHANGELOG.md.
As this would at least stop the tracking of one of my sites by default. (correction: I just remembered that it doesn't use file:// URLs anymore)
|
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
github-actions
bot
added
the
Stale
fix esjint
peterhashair
added
Needs Review
add apply first
|
build js |
There was a problem hiding this comment.
Tested with locally saved html file that loads the Matomo tracking code via https, by default no tracking requests are sent. With the new enableFileTracking config setting enabled the tracking requests are sent 👍
- Functional review done
- Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- Security review done
- Wording review done
- Code review done
- n/a Tests were added if useful/possible
- None Reviewed for breaking changes
- n/a Developer changelog updated if needed
- n/a Documentation added if needed
- n/a Existing documentation updated if needed
|
@peterhashair did you see/action @Findus23 comment re updating the changelog with this? |
|
@justinvelluppillai will update changelog shortly |
…by default (#19835) * add file protocal option in js add file protocal option in js * rebuilt piwik.js * remove function tests remove function tests * set DoNotTrack to true if file protocol * return empty return empty * move file protocol check after do not track move file protocol check after do not track * rebuilt piwik.js * fix esjint fix esjint * add apply first add apply first * rebuilt piwik.js Co-authored-by: peterhashair <peterhashair@users.noreply.github.com>
Description:
Fixes: #17017
add file:// protocol exclude option in the tracker
add
_paq.push(['enableFileTracking']);will start track protocalfile:Review