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

fix($brower): set the url even if the browser transforms it #14499

Closed
wants to merge 3 commits into from
Closed

fix($brower): set the url even if the browser transforms it #14499

wants to merge 3 commits into from

Conversation

bedag-moo
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Fixes #14427

What is the current behavior? (You can also link to an open issue here)

See #14427

What is the new behavior (if this is a feature change)?

Features should not be affected - unless you consider inifinite digest a feature ;-)

Does this PR introduce a breaking change?

No as far as I know.

Please check if the PR fulfills these requirements

Other information:

While $location expects that $browser stores the URL unchanged, some browsers transform the URL
when setting or defer the acutal update. To work around this, $browser.url() kept the unchanged
URL in pendingLocation. However, it failed to update pendingLocation in all code paths, causing
$browser.url() to sometimes incorrectly report previous URLs, which horribly confused $location.
This fix ensures that pendingLocation is always updated if set, causing url() to report the
current url.

Fixes #14427

While $location expects that $browser stores the URL unchanged, some browsers transform the URL
when setting or defer the acutal update. To work around this, $browser.url() kept the unchanged
URL in pendingLocation. However, it failed to update pendingLocation in all code paths, causing
$browser.url() to sometimes incorrectly report previous URLs, which horribly confused $location.
This fix ensures that pendingLocation is always updated if set, causing url() to report the
current url.

Fixes #14427
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@bedag-moo
Copy link
Contributor Author

I signed the CLA.

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@bedag-moo
Copy link
Contributor Author

It's been two weeks ... is there anything I can do to accelerate the review process, @gkalpak?

@bedag-moo
Copy link
Contributor Author

@petebacondarwin might be best qualified to review this change, since he introduced the pendingLocation variable in 8d39bd8. Could you take a look?

@petebacondarwin
Copy link
Contributor

I'll take a look next week

@petebacondarwin petebacondarwin self-assigned this May 13, 2016
@petebacondarwin
Copy link
Contributor

@bedag-moo - really nice PR. $location/$browser have some of the most complicated brain-bending bits of Angular bugs. I think this looks good so I am going to merge it into master and 1.5.x

@petebacondarwin petebacondarwin modified the milestones: 1.5.6, Backlog May 20, 2016
petebacondarwin pushed a commit that referenced this pull request May 20, 2016
While $location expects that $browser stores the URL unchanged, "some browsers" transform the URL
when setting or defer the acutal update. To work around this, $browser.url() kept the unchanged
URL in pendingLocation.

However, it failed to update pendingLocation in all code paths, causing
$browser.url() to sometimes incorrectly report previous URLs, which horribly confused $location.

This fix ensures that pendingLocation is always updated if set, causing url() to report the
current url.

Fixes #14427
Closes #14499
@Martinspire
Copy link

Martinspire commented May 27, 2016

Isn't there a typo in the title and now in the changelog? brower instead of browser?

@petebacondarwin
Copy link
Contributor

Fixed the CHANGELOG at f58d4fb
Thanks @Martinspire

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.