Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(location): correctly rewrite Html5 urls #2485

Closed

Conversation

petebacondarwin
Copy link
Contributor

This commit to master - 58ef323 - introduced a regression where deep link Html5 urls were being rewritten with the part after the appBase being removing.
For example:

http://localhost:8000/build/docs/api/ng.directive:input.number

was always being rewritten to

http://localhost:8000/build/docs/

This was evident when trying to run the documentation on a local machine. These commits fix that.

@nil4
Copy link

nil4 commented Apr 23, 2013

It seems the breakage in HTML5 mode is more serious; see https://gist.github.com/nil4/5444928 for an example. I have hosted the files in that gist in a local webserver under http://ng.dev/en/test/ (you'll need to adjust the links and the prefix variable if you change the root path).

Note that clicking any link fails to match the correct route although all URLs are root-relative; moreover, clicking the first link seems to permanently break the routing. That is, instead of the expected partial.html view being displayed, all links show notfound.html instead.

Furthermore, note that $location.path() reports all page URLs without the /en/test prefix. I have tried to apply your patch to AngularJS v1.1.5-2c34596, but it did not fix the issue.

[Update]: I have updated the gist to fix double-slashes in the route definition, and verified that it works as expected using the official 1.1.3 and 1.1.4 Angular releases.

@mhevery
Copy link
Contributor

mhevery commented Apr 25, 2013

LGTM

@petebacondarwin
Copy link
Contributor Author

Landed at 77ff108

@nil4
Copy link

nil4 commented Apr 26, 2013

Sorry to keep nagging, but please see https://gist.github.com/nil4/5444928 (updated with AngularJS v1.1.5-77ff108). With those files hosted at http://domain/en/test/, $location.path() does not include the /en/test prefix, and as a result, no link matches the configured routes (expected: all links except the last should show the partial.html view, actual: all links show the notfound.html view).
This looks like a regression from v1.1.3 and v1.1.4 where this scenario works as expected.

@mhevery
Copy link
Contributor

mhevery commented Apr 26, 2013

How come there are no tests with this change?

@petebacondarwin
Copy link
Contributor Author

@mhevery Sorry I forgot to squash and it went in as two commits

@petebacondarwin
Copy link
Contributor Author

@nil4 - There is clearly a problem here but I don't yet know the solution. If you have any ideas for the fix then do please post them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants