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

angular $location,$route lifecycle events fired off twice when browser automatically encodes url #8368

Closed
mjj1409 opened this issue Jul 28, 2014 · 8 comments

Comments

@mjj1409
Copy link

mjj1409 commented Jul 28, 2014

issue observed in latest legacy(1.2.21) and beta
versions(1.3.0-beta.17

I provided a simplified example that demonstrates the issue I am encountering below

Basically, the behavior that I am seeing and trying to prevent, is that when my app loads a route whose url is
automatically encoded by the browser (e.g. because there are spaces or +'s in the parameter values), I am seeing the $location and $route service lifecycle events
getting fired off twice.
Using the example provided below the easiest way to duplicate this behavior is to load the app and append the following params:

?message=test val

?message=test+val

If you look at the console output you'll see the first example causes the $locationChangeStart and $locationChangeSuccess handler to be fired off twice.
Based on the documentation I believe angular is monitoring the address bar for changes and I am guessing its firing off these events for the unencoded url and then again
once its encoded.

Is this a bug? Is there a way to prevent this?

NOTE: I also noticed that despite a '+' being a valid substitute for a space in a parameter value, the resulting value retains the '+'; i.e. ?message=test+val will be rendered as 'test+val' not 'test val'. There is an existing ticket for this issue.

   <!DOCTYPE html>
    <html id="ng-app" data-ng-app="test">
    <head>
        <script src="angular.js"></script>
        <script src="angular-route.js"></script>
        <script>
            'use strict';
            angular.module('test', ['ngRoute'])
                .config(function($routeProvider) {
                    $routeProvider.when('/',
                            {
                                template: '{{data.params}}',
                                controller:'TestCtrl',
                                resolve:{params: function($route) {return $route.current.params;}}
                            })
                })
                .run(function($rootScope) {
                    $rootScope.$on('$locationChangeStart',function(event, next, current){
                        console.log(event.name,'current=' + current,'next=' + next);
                    });
                    $rootScope.$on('$locationChangeSuccess',function(event, next, current){
                        console.log(event.name,'current=' + current,'next=' + next,'\n');
                    });
                })
                .controller('TestCtrl',function($scope,params){
console.log('Inside TestCtrl');
$scope.data={params:params};});
        </script>
    </head>
    <body>
        <div ng-view></div>
    </body>
    </html>
@petebacondarwin
Copy link
Contributor

Can you explain a bit why this is a problem for you? Do you have some kind of side-effect in your location change handlers? This is probably not a good idea for GET requests.

I suspect the behaviour is caused because when we encode the URL the address bar is updated which then causes a new location change.

@petebacondarwin petebacondarwin added this to the Backlog milestone Jul 28, 2014
@caitp
Copy link
Contributor

caitp commented Jul 28, 2014

it's potentially a bit of a performance hit at the very least --- but I don't think it should affect routing or anything since the major routers don't care about the search query iirc.

Maybe weigh the cost of encoding the URI before testing if it's changed

@mjj1409
Copy link
Author

mjj1409 commented Jul 28, 2014

Hi,
The undesired side effects are occurring because the $route events are also
fired off twice, which is instantiating the associated controller and its
corresponding resolve functions twice.
In case its helpful, in the case where the parameter value has a space in
it, I am seeing this behavior in all browsers except Firefox. Firefox does
not automatically encode the space to %20.

On Mon, Jul 28, 2014 at 9:41 AM, Pete Bacon Darwin <notifications@github.com

wrote:

Can you explain a bit why this is a problem for you? Do you have some kind
of side-effect in your location change handlers? This is probably not a
good idea for GET requests.

I suspect the behaviour is caused because when we encode the URL the
address bar is updated which then causes a new location change.


Reply to this email directly or view it on GitHub
#8368 (comment).

@caitp
Copy link
Contributor

caitp commented Jul 28, 2014

so we are actually instantiating controllers twice in this case? with ngRoute or ui-router? I recall we're only looking at $location.path() in ngRoute, if I'm not mistaken.

Could you provide a simple reproduction to verify that? If that is the case, then it's definitely something to fix.

@mjj1409
Copy link
Author

mjj1409 commented Jul 28, 2014

I added a console.log statement to the controller code.

@caitp
Copy link
Contributor

caitp commented Jul 28, 2014

http://plnkr.co/edit/TDPmv2qhGl6klOyLcKwY?p=preview I can reproduce this --- I mean, not with the automatic re-encoding of parameters, but it's just weird that we updateRoute just because search changes. @IgorMinar is this desirable behaviour?


Can reproduce the "re-encoding" thing too --- just manually enter ?search=foo bar, the location changes once, and then changes again after the browser automatically encodes it as foo%20bar (chrome)

@btford btford removed the gh: issue label Aug 20, 2014
@JohnLonginotto
Copy link

I get this even without encoding. I have the following code in my controller to keep the user-session in the search param in my URL at all times, even if the routing doesnt contain it (desirable because i WANT users to share sessions easily by copy/pasting address bar, so i dont store this in a cookie):

$scope.$on('$locationChangeStart', function( event ) { $location.search({session: userSession}); });

But this always fires twice when user does some routing. If the user navigates to /newPage, the above will intercept and put in a ?session=a7h3k1 or something in the URL. This fires off ANOTHER $locationChangeStart event (because url changed), which fires off $location.search({session: userSession}). But because this time it DOESNT change anything be re-setting the value, we dont get stuck in an infinite loop.

In other words, this isnt a bug, it works fine - you just have to accept that $locationChangeStart can happen more than once ;)

@gkalpak
Copy link
Member

gkalpak commented Jan 3, 2017

I can't reproduce the issue with re-encoding (neither with 1.2.21, nor with 1.6.1). This is quite old and the original issue is likely to be fixed already. If the issue is still present on latest versions, please open a new bug report providing the exact steps to reproduce the issue.

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

6 participants