fix($location): do not decode forward slashes in the path in HTML5 mode #16316
Conversation
|
Be aware that this does not change the behaviour of hash-bang (non-HTML5 mode) paths. This is because slashes do not need to be escaped once they are in the hash section of the URI and we have tests that expect encoded slashes to be decoded into the path when parsed by The use case in #16312 is very unlikely to use hashbang mode since it is a hybrid AngularJS/Angular app that would run on browsers that support HTML5 mode. |
src/ng/location.js
Outdated
|
|
||
| function decodePath(path, html5Mode) { | ||
| var segments = path.split('/'), | ||
| i = segments.length; |
petebacondarwin
force-pushed
the
issue-16312-matrix-params
branch
from
c3d18b2
to
f66a191
Compare
petebacondarwin
force-pushed
the
issue-16312-matrix-params
branch
from
f66a191
to
f248563
Compare
|
@gkalpak You're so right! The change from decodeURIComponent to decodeURI on line 425 broke our URL matcher type in ui-router since decodeURI doesn't decode %2F to / in the path (we don't use html5-mode). I would like to see this one line reverted, but I'm not sure what the implications are... Please let me know if you need any more info from me. |
@veloek Fwiw: I guess, for the time being, you can roll back to 1.6.6 and lock your angularjs version. |
|
@frederikprijck Yup, that's what I did :) |
|
@veloek - can you provide a reproduction of the regression for us? |
|
@petebacondarwin I'll see if I get the time :) I guess the locationPrototype.url function is used in both html5mode and "hash-mode", right? |
|
Hmm, I see. We probably need to shift some of that logic into the different implementations: |
|
Here is a fix for the regression: #16350 |
Closes #16312
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
bug fix
What is the current behavior? (You can also link to an open issue here)
See #16312
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change?
It depends what people expected of the location escaping...
I think what we had before was buggy. So no.
Please check if the PR fulfills these requirements
Other information: