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

$location issue for decoded and unencoded urls #16100

Open
1 of 3 tasks
dmartres opened this issue Jul 11, 2017 · 4 comments
Open
1 of 3 tasks

$location issue for decoded and unencoded urls #16100

dmartres opened this issue Jul 11, 2017 · 4 comments

Comments

@dmartres
Copy link

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:

Hi guys.
After a discussion in a PR that I've created to fix an error in $location service for url that contains apostrophes @gkalpak notice that potencially apostrophes in url is not the main problem instead the problem is using decoded and undecoded urls.

see the PR for more details. #16098

Expected / new behavior:
undecoded and decoded url should work using $location service

Angular version: 1.x.

Browser: all

Anything else:

How to fix it

In src/ng/browser.js there is a function fireStateOrUrlChange which has this validation

if (lastBrowserUrl === self.url() && prevLastHistoryState === cachedState) {{
return;
}
The main problem seems to be that we are comparing self.url() to lastBrowserUrl, but lastBrowserUrl can be set from different sources, not always with the same encoding/decoding

@jbedard
Copy link
Contributor

jbedard commented Jul 26, 2017

@gkalpak you mean (from your comment) doing something such as url = urlResolve(url).href within $browser.url or changing all calls to $browser.url(val) to be consistent? Adding the urlResolve seems to work, although changes a few tests (some URLs now end in /).

jbedard added a commit to jbedard/angular.js that referenced this issue Jul 26, 2017
@gkalpak
Copy link
Member

gkalpak commented Jul 26, 2017

@jbedard, I didn't think through the implementation details. I was referring to the end result 😁

@jbedard jbedard self-assigned this May 3, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 22, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Jun 24, 2018
@jbedard
Copy link
Contributor

jbedard commented Aug 13, 2018

@dmartres can you confirm if this is still an issue in 1.7.3? I think aee7d53 (fixing #16592) may have fixed this?

@jbedard jbedard modified the milestones: Backlog, 1.7.x Sep 26, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 2, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 3, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 16, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 16, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 16, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 25, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Oct 29, 2018
jbedard added a commit to jbedard/angular.js that referenced this issue Nov 21, 2018
Calls to `$browser.url` now normalize the inputted URL ensuring multiple
calls only differing in formatting do not force a browser `pushState`.

Normalization is done the same as the browser location URL and may
differ per browser and may be changed by browsers. Today no browsers
fully normalize URLs so this does not fix all instances of this issue.

See angular#16100
Closes angular#16606
jbedard added a commit that referenced this issue Nov 21, 2018
Calls to `$browser.url` now normalize the inputted URL ensuring multiple
calls only differing in formatting do not force a browser `pushState`.

Normalization is done the same as the browser location URL and may
differ per browser and may be changed by browsers. Today no browsers
fully normalize URLs so this does not fix all instances of this issue.

See #16100
Closes #16606
Narretz pushed a commit that referenced this issue Nov 26, 2018
Calls to `$browser.url` now normalize the inputted URL ensuring multiple
calls only differing in formatting do not force a browser `pushState`.

Normalization is done the same as the browser location URL and may
differ per browser and may be changed by browsers. Today no browsers
fully normalize URLs so this does not fix all instances of this issue.

See #16100
Closes #16606
@jbedard
Copy link
Contributor

jbedard commented Dec 14, 2018

We've partially fixed this using the browser <a href> to normalize URLs in a few key areas (e68697e, 2f72a69). However no browser normalizes everything with this method. To truly fix this we would need to implement the normalizing ourselves.

I implemented a POC to do that normalization manually, however we decided this is too big of a change at this point. FYI 15 of the the tested added in that commit currently fail in chrome.

For these reasons we are going to put this in a "wont fix" state.

@jbedard jbedard modified the milestones: 1.7.x, 1.7.x - won't fix Dec 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants