$location.state problems in 1.3.0-rc.5 #9545

bukharin · 2014-10-10T06:46:58Z

My application stores some data in history state object. Before rc.5 version I used history.replaceState method. Now I rewrite my code to use new API: $location.state(obj).replace()

For instance:

$rootScope.$on('$locationChangeSuccess', function() {
       if(!$location.state())
              $location.state(Math.random()).replace();
});

It seems works fine, but when I click 'back' browser button (popstate event) - after that application stops responding on any link clicks (routes does not changes).

After investigate the source code I found that condition at angular.js:10905 always falsy

if (initializing || oldUrl !== $location.absUrl() ||
          ($location.$$html5 && $sniffer.history && oldState !== $location.$$state)) {
...
}

oldState and $location.$$state are equals after user click on the link

This problem in Chrome and FF. IE11 throws an exception after bootstrap:

Error: [$rootScope:infdig] 10 $digest() iterations reached. Aborting!
Watchers fired in the last 5 iterations: []
http://errors.angularjs.org/1.3.0-rc.5/$rootScope/infdig?p0=10&p1=%5B%5D
   at Scope.prototype.$digest (https://localhost:44300/vendor/angularjs/angular.js:13731:13)
   at Scope.prototype.$apply (https://localhost:44300/vendor/angularjs/angular.js:13955:13)
   at bootstrapApply (https://localhost:44300/vendor/angularjs/angular.js:1486:9)
   at invoke (https://localhost:44300/vendor/angularjs/angular.js:4133:7)
   at doBootstrap (https://localhost:44300/vendor/angularjs/angular.js:1484:5)
   at bootstrap (https://localhost:44300/vendor/angularjs/angular.js:1504:5)
   at Global code (https://localhost:44300/scripts/base/app.js:100:1)
SCRIPT5022: [$rootScope:infdig] 10 $digest() iterations reached. Aborting!
Watchers fired in the last 5 iterations: []
http://errors.angularjs.org/1.3.0-rc.5/$rootScope/infdig?p0=10&p1=%5B%5D

The text was updated successfully, but these errors were encountered:

IgorMinar · 2014-10-10T17:23:11Z

@mzgol can you take a look at this?

mgol · 2014-10-11T19:41:23Z

From what I see the issue is caused by the fact that history.state might be a primitive, not an object. In case of objects, pressing the back button will give us a state object deep equal to the previous one at this point but referentially different which will work fine; it won't for primitives like number from Math.random() here.

I'll need to think how to bypass this issue.

@bukharin Can you confirm that if you don't use numbers as state objects but objects (try e.g. passing {number: Math.random()} instead of Math.random() the issue disappears?

mgol · 2014-10-11T21:05:49Z

Actually, I'm not sure what the issue is. What I mentioned above is true but I don't see how it can cause issues.

@bukharin could you post a minimal code example showing the problem so that I fully understand what's the issue? The check in location.js first checks for a URL so both URLs and states have to be identical for the if to evaluate to false; I need more context.

mgol · 2014-10-11T21:08:50Z

I see the IE11 issue in my app so I'll try to debug that first.

mgol · 2014-10-11T22:14:28Z

As for IE11, I'm taking my jaw from the floor... In this browser, the following:

history.pushState({a: 2}, 'a', 'b');
var o = history.state;
console.log(o === history.state);

prints false... O_o. This means the entire logic of comparing states referentially won't work and we have to find a different approach.

@IgorMinar I don't think I can fix it before Monday. I should've notice this before but $browser in location tests is mocked and obviously doesn't have this bug.

EDIT: I submitted this to IE bug tracker: https://connect.microsoft.com/IE/feedback/details/998728.

@bukharin Thanks for the report! It seems we actually have 2 bugs here; I'm focusing on the IE one now since it's more severe but I'd still love to get a test case to be able to fix it in Chrome/Firefox later.

mgol · 2014-10-11T22:33:36Z

Note to self: try to avoid the issue by caching history.state on popstate/hashchange and comparing to that instead of history.state.

EDIT: that idea probably won't work since:

history.pushState({a: 2}, 'a', 'b');
history.state === history.state; // false

It seems the object is deserialized on read so every read will get sth different.

mgol · 2014-10-11T22:48:48Z

@IgorMinar The only workaround I see is to attach the internal $$id property to every non-primitive history.state and compare that if the strict equality fails. Is it acceptable?

EDIT: we could also attach this $$id thing only in IE.

bukharin · 2014-10-13T03:19:45Z

@mzgol I try to create sample fiddle to demonstrate the problem: http://jsfiddle.net/ckhhqko3/1/

Steps to reproduce:

Navigate between page1 and page2
Click on the 'back' button
Try to navigate
Application not responding on link navigation

IgorMinar · 2014-10-13T08:15:22Z

@mzgol so how about we go back to the wrapper object idea in which case the wrapper would be {id: someId, data: stateObject} ?

mgol · 2014-10-13T10:55:45Z

@IgorMinar The wrapper would have to be saved in history.state instead of explicitly what user has passed so I'd like to avoid it if possible.

I was wrong, though, caching works. After all, in all browsers creating a new history entry and going back will create a different instantiation of history.state than before (though equal referentially). So the only problem is that we want to have stable history.state when not navigating and that can be achieved with caching.

A PR to follow.

IE 10-11+ deserialize history.state on every read, causing simple comparisons against history.state always return false. Account for that caching `history.state` on every hashchange or popstate event. Also, prevent firing onUrlChange callbacks twice if both popstate and hashchange event were fired. Closes angular#9587 Refs angular#9545

mgol · 2014-10-13T11:32:40Z

PR for the IE issue: #9587. I still need to investigate the other part of this issue.

IE 10-11+ deserialize history.state on every read, causing simple comparisons against history.state always return false. Account for that caching `history.state` on every hashchange or popstate event. Also, prevent firing onUrlChange callbacks twice if both popstate and hashchange event were fired. Closes angular#9587 Refs angular#9545

IE 10-11+ deserialize history.state on every read, causing simple comparisons against history.state always return false. Account for that caching `history.state` on every hashchange or popstate event. Also: 1. Prevent firing onUrlChange callbacks twice if both popstate and hashchange event were fired. 2. Fix the issue of routes sometimes not firing the URL change in all browsers. Closes angular#9587 Fixes angular#9545

mgol · 2014-10-13T13:52:49Z

I might have been able to fix the issue, I need to think how to write a test for it.

@bukharin Could you check that the issue is fixed with angular.js from https://code.angularjs.org/snapshot/angular.js and angular-route.js from https://code.angularjs.org/snapshot/angular-route.js?

Thanks for the test case! It helped a lot.

EDIT: URLs to angular files changed.

IE 10-11+ deserialize history.state on every read, causing simple comparisons against history.state always return false. Account for that caching `history.state` on every hashchange or popstate event. Also: 1. Prevent firing onUrlChange callbacks twice if both popstate and hashchange event were fired. 2. Fix the issue of routes sometimes not firing the URL change in all browsers. Closes angular#9587 Fixes angular#9545

bukharin · 2014-10-14T03:56:55Z

@mzgol It seems ok, thanks

bukharin · 2014-10-15T04:40:36Z

@mzgol $location.state(obj).replace() method trigger $routeChangeSuccess and $locationChangeSuccess events. Is it ok?

mgol · 2014-10-15T14:31:02Z

@bukharin

$location.state(obj).replace() method trigger $routeChangeSuccess and $locationChangeSuccess events. Is it ok?

It's intended. States and URLs are changed by the browser at the same time. If we decided that we shouldn't trigger those events on $location.state(obj).replace() since there are no new history entries and only the state has changed, we'd still need to consider $location.state(newState) (without .replace()) - this will create a new history entry with the same URL; should we trigger $locationChangeSuccess then? I think we should as we have a new history entry. But it would be weird to trigger the event in all the following cases:

$location.url(newUrl)
$location.url(newUrl).replace()
$location.url(newUrl).state(newState)
$location.url(newUrl).state(newState).replace()
$location.state(newState)

and not trigger it/them only in case of $location.state(newState).replace().

bukharin · 2014-10-16T03:40:11Z

@mzgol I think that $location.state() should not trigger this events, because actually this method do not change route/location. May be $location.state() should trigger $routeUpdate event? Or create a new events for this one?

mgol · 2014-10-16T12:42:39Z

@bukharin $location.state(newState) doesn't change the location.href string but it does add a new history entry with the same URL.

Anyway, 1.3.0 final is out and any change to those events would be a breaking change so I don't think anything can be changed here. cc @IgorMinar

You can easily handle it on your side, can't you?

tbosch assigned tbosch and mgol and unassigned tbosch Oct 10, 2014

IgorMinar added this to the 1.3.0-rc.6 milestone Oct 10, 2014

IgorMinar added type: bug component: $location frequency: low severity: broken expected use labels Oct 10, 2014

mgol mentioned this issue Oct 13, 2014

fix($browser): account for IE deserializing history.state on each read #9587

Closed

mgol closed this as completed in 1efaf3d Oct 13, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

$location.state problems in 1.3.0-rc.5 #9545

$location.state problems in 1.3.0-rc.5 #9545

bukharin commented Oct 10, 2014

IgorMinar commented Oct 10, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

bukharin commented Oct 13, 2014

IgorMinar commented Oct 13, 2014

mgol commented Oct 13, 2014

mgol commented Oct 13, 2014

mgol commented Oct 13, 2014

bukharin commented Oct 14, 2014

bukharin commented Oct 15, 2014

mgol commented Oct 15, 2014

bukharin commented Oct 16, 2014

mgol commented Oct 16, 2014

$location.state problems in 1.3.0-rc.5 #9545

$location.state problems in 1.3.0-rc.5 #9545

Comments

bukharin commented Oct 10, 2014

IgorMinar commented Oct 10, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

mgol commented Oct 11, 2014

bukharin commented Oct 13, 2014

IgorMinar commented Oct 13, 2014

mgol commented Oct 13, 2014

mgol commented Oct 13, 2014

mgol commented Oct 13, 2014

bukharin commented Oct 14, 2014

bukharin commented Oct 15, 2014

mgol commented Oct 15, 2014

bukharin commented Oct 16, 2014

mgol commented Oct 16, 2014