fix(angular_1_router): ng-link is generating wrong hrefs#7423
fix(angular_1_router): ng-link is generating wrong hrefs#7423davidreher wants to merge 4 commits into
Conversation
At the moment ng-link is generating html5mode URLs for `href`s. Instead it should check whether or not html5mode is enabled and create the `href`s accordingly. The renaming in the `getLink` function is aligning it to `RouterLink`'s `_updateLink`.
|
I think you are right that this is a bug @davidreher. I think we can tighten up the implementation a bit. |
| Location.prototype.prepareExternalUrl = function(url) { | ||
| if (url.length > 0 && !url.startsWith('/')) { | ||
| url = '/' + url; | ||
| } |
There was a problem hiding this comment.
Should we always prepend a /?
Previously, HTML5 mode URLs were prepended with ./
There was a problem hiding this comment.
You are right about the ./ I will fix that
There was a problem hiding this comment.
That being said it seems that the A2 Location may not do this so I am wondering which is wrong...
There was a problem hiding this comment.
Did you already find out ;) ?
Conflicts: modules/angular1_router/src/ng_outlet.ts modules/angular1_router/test/ng_link_spec.js
|
@petebacondarwin the tests are now run with html5mode enabled and disabled. What about the base href? Any preferences how it should be set? Btw. I am working on the failing tests, I need to setup a linux vm since windows build (and test) is not working atm (see #7457) |
|
@petebacondarwin any ideas how to determine whether html5mode is enabled or not? In our application (angular 1.5) accessing |
|
I have been working on this today. The problem with the tests is that |
|
Have a look at the last commit of #7489 |
|
Yes. good idea. Now to think about baseHref... |
|
@Mischi suggested to simply insert the |
|
I am not sure if that would mess with karma loading up files... |
|
I've just hit this bug. This is a blocker to migrating production apps using hashbang mode to the new router as all links would be broken if you right-clicked and pressed "Open in new tab". |
|
@petebacondarwin how do we proceed? Shall I commit 0382d80 and 8869e82 to your pull request? What do we do about the |
|
OK, so I will merge your commits into my PR and add tests for base href changes... |
At the moment ng-link is generating html5mode URLs for `href`s. Instead it should check whether or not html5mode is enabled and create the `href`s accordingly. The renaming in the `getLink` function is aligning it to `RouterLink`'s `_updateLink`. Closes angular#7423
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
At the moment ng-link is generating html5mode URLs for
hrefs. Instead it should check whether or not html5mode is enabled and create thehrefs accordingly.The renaming in the
getLinkfunction is aligning it toRouterLink's_updateLink.cc: @btford @petebacondarwin