Query params containing commas result in an extra navigation

[bourey]

I'm submitting a ... (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior
When visiting an ng2 URL with a query parameter value containing a comma, ng2 redirects to the same path with all query parameters removed.

Expected behavior
Parameters should not disappear, and the router should properly report the value for the query param.

Minimal reproduction of the problem with instructions
Steps to reproduce:

1. Download https://github.com/bourey/ngupgrade-example/tree/ng2
2. npm start
3. npm install
4. Visit http://localhost:3000/teams?foo=bar,baz

What is the motivation / use case for changing the behavior?
Our product currently has some parameter values containing commas, and we need to support them.

Please tell us about your environment:

• Angular version: 4.0.2
• Browser: Chrome
• Language: TS

[vicb]

The first navigation is triggered by the Angular2 router

Subsequent navigations are initiated by the interop, most specifically:

function setUpLocationSync(ngUpgrade) {
    if (!ngUpgrade.$injector) {
        throw new Error("\n        RouterUpgradeInitializer can be used only after UpgradeModule.bootstrap has been called.\n        Remove RouterUpgradeInitializer and call setUpLocationSync after UpgradeModule.bootstrap.\n      ");
    }
    var router = ngUpgrade.injector.get(_angular_router.Router);
    var url = document.createElement('a');
    ngUpgrade.$injector.get('$rootScope')
        .$on('$locationChangeStart', function (_, next, __) {
        url.href = next;
        router.navigateByUrl(url.pathname);
    });
}

from the router-upgrade

[vicb]

Seems like changing router.navigateByUrl(url.pathname); for router.navigateByUrl(url.pathname + url.search); fixes the issue

[bourey]

While query parameters aren't dropped anymore, it looks like they're still not being handled properly. Now I see a brief flicker in the location bar, which appears to correspond to the URL quickly changing to http://localhost:3000/teams?foo=bar2%2baz, then back to http://localhost:3000/teams?foo=bar,baz.

The browser's back button also doesn't successfully navigate back in the URL history for parameters containing a comma. To demonstrate this, try the original repro case, but add queryParamsHandling="merge" to the link in team_list.component.html. If you click on a team link, then attempt to use the browser's "back" functionality, the page is not updated.

[naomiblack]

re-opening this one. @vicb @mhevery @jasonaden

[KKrisu]

One addition from me: The setUpLocationSync function has another issue - it drops the hash part of the url.
So the fix probably should be router.navigateByUrl(url.pathname + url.search + url.hash);

cc @vicb @mhevery @jasonaden @naomiblack