mergeUrlParams wrong with fragment url
Summary
mergeUrlParams
is wrong if the URL has a fragment. The query will be placed after the fragment identifier (#frag?k=v
) instead of before the hash (?k=v#frag
). This affects for example the "Hide whitespace changes" button after following a comment link (#note_XXXX
).
Steps to reproduce
- Visit Activity page
- Click any "Commented on merge request" link
- Click on "Changes"
- Click on "Hide whitespace changes"
Example Project
What is the current bug behavior?
URL becomes (example):
https://gitlab.com/inkscape/inkscape/merge_requests/418/diffs#note_118090002?w=1
What is the expected correct behavior?
URL becomes (example):
https://gitlab.com/inkscape/inkscape/merge_requests/418/diffs?w=1#note_118090002
Possible fixes
Broken mergeUrlParams
function:
https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/lib/utils/url_utility.js#L19
Possible better implementation:
function mergeUrlParams(params, url) {
const re = /^([^?#]*)(\?[^#]*)?(.*)/;
let merged = {};
let urlparts = url.match(re);
if (urlparts[2]) {
urlparts[2].substr(1).split('&').forEach(function(part) {
if (part.length) {
let kv = part.split('=');
merged[decodeURIComponent(kv[0])] =
decodeURIComponent(kv.slice(1).join('='));
}
});
}
Object.assign(merged, params);
let pstrings = Object.keys(merged).map(
key => encodeURIComponent(key) + '=' + encodeURIComponent(merged[key]));
return urlparts[1] + '?' + pstrings.join('&') + urlparts[3];
}
Edited by Thomas Holder