Issue: 2nd ?/& appended to cross domain link containing anchor #14383
Conversation
…ed and a cross domain link contains an anchor reference (#)
| @@ -5264,12 +5264,6 @@ if (typeof window.Piwik !== 'object') { | |||
| // and visitorId (eg userId might be set etc) | |||
| link = removeUrlParameter(link, configVisitorIdUrlParameter); | |||
|
|
|||
| if (link.indexOf('?') > 0) { | |||
There was a problem hiding this comment.
would you still need to do this logic when there is no hash in the URL?
There was a problem hiding this comment.
No - the logic within the addUrlParameter function duplicates the removed logic, but applies to the baseURL as opposed to the full url:
if (baseUrl.indexOf('?') === -1) {
baseUrl += '?';
} else if (!stringEndsWith(baseUrl, '?')) {
baseUrl += '&';
}
// nothing to if ends with ?
The baseURL variable will be the same as the url variable in the case that there are no hashes in the url.
As the addUrlParameter function is always called from the replaceHrefForCrossDomainLink directly after the removed logic, then the inclusion of this logic is superfluous in all cases.
I have tested the 4 use cases identified under Example Destination Links in the description above, to ensure that they are processed correctly to generate the Expected Results
|
Looks good to merge but need to find time to merge as we need to update the piwik.js/matomo.js later as it currently fails due to some line break (https://travis-ci.org/matomo-org/matomo/jobs/526012825#L966) ... happens when generating the minified version eg on Mac compared to Linux. Bit annoying and need to tweak the test at some point to ignore this line break at the end. |
|
Thank you for the report & the detailed PR @danlance - Appreciated! 👍 |
Description
When cross domain linking is being used, and tracked cross domain URL contains an anchor (#), an additional question mark (?) / ampersand (&) symbol is appended to the end of the destination URL.
This creates an invalid link, causing the anchor to fail, and can potentially cause the link to be processed incorrectly by the destination web server.
Conditions:
Example Destination Links
Expected Results:
Actual Results:
Cause:
Fix:
This change is included in the pull request, as well as minifying the changed js using YUICompressor 2.4.8. as per the instructions contained within the ReadMe. (Please note, this was performed on Mac OSX not Linux)