fixes #16859
Still need to add documentation.
Is it correct to sendRequest immediately?
Are there other parameters we need to take care of?
Is it correct to sendRequest immediately?
@geekdenz it wouldn't be a separate request immediately but the information would be sent along the next tracking request. It's mentioned in the issue.
@geekdenz I guess this might need to be implemented in another way. The setPagePerformanceTiming
method might only need to set some internal variables for the performance metrics. And in sendRequest
we check if custom values are provided and append them in that case and if not fall back to the auto detection. See
https://github.com/matomo-org/matomo/blob/da4df21e808fa983e32c6402c8f3de90145e0910/js/piwik.js#L3773-L3777
When appending custom values we also need to set performanceTracked = true
to ensure the values aren't tracked twice.
Maybe we should also set performanceTracked = false
in setPagePerformanceTiming
to ensure the values are tracked again when some updates them.
Only failure is now this:
https://app.travis-ci.com/github/matomo-org/matomo/jobs/533572041#L1120-L1122
Looks like it's not caused by my. So OK to merge?
Added some more tests here:
https://github.com/matomo-org/test-examples/commit/33c6d3cabcf5d14b6e55bfe1d1114c712a2dbfaa
Please see the updated branch at https://github.com/matomo-org/test-examples/tree/trackingpageperformance to ease testing for this.
Before reviewing, maybe give me another chance to check some details tomorrow (2021-09-08). I'll comment once I've rechecked some old manual checks I have not done yet. Might save us all some time.
Not 100% sure but I think this test failure is probably not related to this issue:
https://app.travis-ci.com/github/matomo-org/matomo/jobs/536091064#L1112-L1128
Maybe something a reviewer would want to consider before approving.
Considerations that went into this:
undefined
if intendedI think there are still some improvements that should be made here before merging.
Documented here:
https://github.com/matomo-org/developer-documentation/pull/549
Saw that these seem fine to download:
https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/48972/
Not sure which change made these fail, but it seems not related to this PR.
Not sure which change made these fail, but it seems not related to this PR.
@geekdenz Those look like some random failures. Might be good to merge in the latest changes from 4.x-dev into your branch as on 4.x-dev all tests are currently passing.
@tsteur Your last commit actually would have required another build of the javascript files. Will fix that directly on 4.x-dev