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
If the custom URL is set, check if is file protocol. #20056
Conversation
build js |
build js |
@peterhashair I would be awesome to also add some tests for that new config flag and custom file urls not being tracked |
@sgiehl I was trying to add tests, but it seems like Travis didn't allow swap protocol, any suggestions? |
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
check current url checks
format alien code
update regex
@bx80 I've fixed the code and added some tests. This should be ready for a final review now. |
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.
File protocol handling tests well for me 👍
Only thing I noticed was that the count tracking events test is failing, this PR updated the count to 59, but there are 62 events in my local tests. This count is also wrong on 4.x-dev too so it might be my config?
- 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
- n/a Wording review done
- Code review done
- Tests added Tests were added if useful/possible
- Reviewed for breaking changes
- n/a Developer changelog updated if needed
- n/a Documentation added if needed
- Existing documentation updated if needed
Description:
Fixes: #20049
If a custom URL is set, check if is file protocol.
Review