Fix: Router ignored the root when creating hrefs

[rluba]

Aurelia router generated absolute paths but did not append the root path (ie. <base href="…">). This is incorrect and caused all non-default click actions (eg. CMD+Click, Right-click-> "Open in new Tab/Browser", etc.) to navigate to the wrong URL (see issue #457) and also broke links for crawlers.

Normal clicks only worked because they are intercepted by Aurelia and forwarded to aurelia-browser-history, which contains a corresponding bug that treats all URLs as fragments instead of expecting the base path to be present in absolute paths.

This fix requires the corresponding fix for aurelia-browser-history or the history will forward incorrect fragments to the router.

[CLAassistant]

All committers have signed the CLA.

[rluba]

Don’t merge this PR yet.
I’ve discovered that this solution doesn’t work in all cases and that there are more issues when root is set to something else than /. I’ll test it more thoroughly and keep you updated.

[rluba]

I discovered that redirects via route configurations, eg., {route: '', name: 'home', redirect: 'somepath'}, are broken when using my PR. But while trying to fix this, I discovered that these redirects are generally broken on master (as of 5fcb6f4).

There are multiple issues:

1. Since 487c33d: TypeError: Cannot read property 'childRouter' of undefined is thrown because determineWhatToLoad(…) tries to interpret the Redirect plan as a regular plan .
2. Since aafa070: _buildNavigationPlan(…) calls _createNavigationInstruction(…) and then generate(…) on a router before it knows it’s baseURL, causing it to pass incorrect URLs to new Redirect(…). This probably cancels out when there’s no root set, but it prevents me from fixing the redirect when a root is set.

Are these issues already known?

[rluba]

1. is fixed in a separate PR (Fix error introduced in 487c33d5 (when trying to fix #480) #603)
2. is fixed in 0737128

The test coverage for this module is pretty horrible. I’ll help bring it up in the future if we decide to stick with Aurelia.

[davismj]

The test coverage for this module is pretty horrible. I’ll help bring it up in the future if we decide to stick with Aurelia.

That is a generous assessment lol. Part of the reason Router dev has been slow recently is I find myself spending more time fixing the tests than getting to actual issues, and its very time consuming.

Thanks for your hard work. These are on my radar.

[mreiche]

Can someone please update this PR and merge it finally? This requirement is still unsatisfied.

[bigopon]

@mreiche We are waiting for this PR at #632 to get merged, and then I'll incorporate @rluba changes in a new PR, or would be even better if @rluba could create PR based on refactored code.

[mreiche]

#632 has been merged a while ago.