feat($location): add support for History API state handling #9027

mgol · 2014-09-11T04:31:25Z

This is a continuation of #3325, re-created per my e-mail thread with @IgorMinar.

EDIT: new commit message:

Adds $location state method allowing to get/set a History API state via
pushState & replaceState methods.

Note that:

Angular treats states undefined and null as the same; trying to change
one to the other without touching the URL won't do anything. This is necessary
to prevent infinite digest loops when setting the URL to itself in IE<10 in
the HTML5 hash fallback mode.
The state() method is not compatible with browsers not supporting
the HTML5 History API, e.g. IE 9 or Android < 4.0.

Adds $location pushState & replaceState methods acting as proxies to history pushState & replaceState methods. This allows using pushState to change state and title as well as URL. Note that these methods are not compatible with browsers not supporting the HTML5 History API, e.g. IE 9. Closes angular#9027

mgol · 2014-09-11T04:34:46Z

The location.js file has more changes but they're mostly trivial.

mgol · 2014-09-11T04:36:47Z

@IgorMinar I'm still not sure what to think about the pushState title handling. I created $location.pushState(state, title, url) signature so that it matches native history.pushState but AFAIK every browser simply ignores the title argument so I'm not sure how useful it's to pass it around.

You'd think the title param changes the title of the page or something but nope. Web standards.

Adds $location pushState & replaceState methods acting as proxies to history pushState & replaceState methods. This allows using pushState to change state and title as well as URL. Note that these methods are not compatible with browsers not supporting the HTML5 History API, e.g. IE 9. Closes angular#9027

mgol · 2014-09-11T05:13:25Z

src/ng/browser.js

    // Android Browser BFCache causes location, history reference to become stale.
    if (location !== window.location) location = window.location;
    if (history !== window.history) history = window.history;

    // setter
    if (url) {
-      if (lastBrowserUrl == url) return;
+      if (lastBrowserUrl == url && (!$sniffer.history || equals(lastBrowserState, state))) {


@IgorMinar IIRC this if was originally meant to prevent infinite digest in IE9 in hash mode. I added comparing states, I could also check title (though browsers don't expose the title param used in pushState in any way) but I'm not sure if it's needed in HTML5 mode?

no need to use equals here. we can just compare references since browsers always create new instance of history.state when state change occurs.

I think you are right. this stuff should not be needed at all. let's remove it until we see a need for it.

OK, I'm leaving it as it was then as it's for the hash mode only anyway.

Hmm, actually, I need to be able to do $location.pushState(state, '', 'the-same-url') so I need to have a relaxed condition (I've already written a test for that). Ideally I'd just skip the URL check here in HTML5 mode but $browser doesn't know about such a concept. I'll think about it.

For reference, this logic was added in ffb2701; am I right that it affects only the hash-fallback-in-html5-mode case?

Anyway, while it was added because of oldIE, people may depend on the fact they can call $location.url('the-same-url') and expect nothing to happen in such a case. I might skip this logic if $sniffer.history but then the behavior would depend on whether the browser supports the History API or not which is not ideal.

Ideally I'd skip this logic only if the change originated via $location.pushState/replaceState, but I can't detect that from within $browser, mainly because history.pushState(undefined, '', 'new-url') is still valid so the basic state === undefined check won't be enough.

This hash-in-html5-mode fallback is a real PITA. :)

Adds $location pushState & replaceState methods acting as proxies to history pushState & replaceState methods. This allows using pushState to change state and title as well as URL. Note that these methods are not compatible with browsers not supporting the HTML5 History API, e.g. IE 9. Closes angular#9027

mgol · 2014-09-23T14:45:58Z

@IgorMinar any remarks?

Adds $location pushState & replaceState methods acting as proxies to history pushState & replaceState methods. This allows using pushState to change state and title as well as URL. Note that these methods are not compatible with browsers not supporting the HTML5 History API, e.g. IE 9. Closes angular#9027

IgorMinar · 2014-09-26T18:34:25Z

src/ng/browser.js

-        else {
-          history.pushState(null, '', url);
-          // Crazy Opera Bug: http://my.opera.com/community/forums/topic.dml?id=1185462
-          baseElement.attr('href', baseElement.attr('href'));


I just merged #8589 so this hunk is now obsolete

mgol · 2014-10-04T16:30:33Z

Note: current design assumes that $location.url(newUrl) will do a pushState/replaceState with the current state, not a null/undefined one. Same applies to $location.state(newState) - the same URL is used.

If both are needed to be changed, one has to invoke $location.url(newUrl).state(newState).

Adds $location state method allowing to get/set a History API state via pushState & replaceState methods. Note that: - Angular treats states undefined and null as the same; trying to change one to the other without touching the URL won't do anything. This is necessary to prevent infinite digest loops when setting the URL to itself in IE<10 in the HTML5 hash fallback mode. - The state() method is not compatible with browsers not supporting the HTML5 History API, e.g. IE 9 or Android < 4.0. Closes angular#9027

mgol · 2014-10-06T11:55:06Z

One note: currently one invocation of $location.state({a: 2}) will cause all subsequent $location.url('newUrl') invocations to trigger history.pushState({a: 2}, '', 'newUrl'), even far in the site future. I could reset the state during digest if that's desired.

IgorMinar · 2014-10-06T18:59:45Z

src/ng/location.js


          $location.$$parse(newUrl);
-          if ($rootScope.$broadcast('$locationChangeStart', newUrl,
-                                    oldUrl).defaultPrevented) {
+          $location.$$state = copy(newState);


Because (at least in Chrome) history.state cannot be swapped but can be augumented:

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

The more important copy is in Location#state, though as then we save the state object passed by the user and nothing prevents them from modifying it afterwards (i'd guess it may be common).

we should add this reasoning into the code as a comment so that we don't forget.

ok, will do

Adds $location state method allowing to get/set a History API state via pushState & replaceState methods. Note that: - Angular treats states undefined and null as the same; trying to change one to the other without touching the URL won't do anything. This is necessary to prevent infinite digest loops when setting the URL to itself in IE<10 in the HTML5 hash fallback mode. - The state() method is not compatible with browsers not supporting the HTML5 History API, e.g. IE 9 or Android < 4.0. Closes angular#9027

mgol · 2014-10-07T02:08:01Z

src/ng/location.js

      var currentReplace = $location.$$replace;
+      var currentStateSent = $location.$$stateSent && changeCounter;


I'm not sure what's the deal with changeCounter but without this check here I was getting into the if below with equal URLs & states and $$stateSent === true which was causing the state to be set to null (& breaking a test at that).

mgol · 2014-10-07T02:21:41Z

@IgorMinar ready for the final review.

mgol · 2014-10-07T04:30:19Z

One weird result is that if we pushed {a: 2} and we want to later push the same object again, checking $location.state() will give us {a: 2} but we still need to invoke $location.state({a: 2}) to have the state set to this object and not null.

IgorMinar · 2014-10-07T06:05:47Z

src/ng/browser.js

-      if (lastBrowserUrl == url) return;
+      // Prevent IE<10 from getting into redirect loop when in LocationHashbangInHtml5Url mode.
+      // See https://github.com/angular/angular.js/commit/ffb2701
+      if (lastBrowserUrl === url && (!$sniffer.history || history.state === state)) {


if this fix is just for ie<10 as you documented then we'll never need to check history.state, no?

updated a comment

Adds $location state method allowing to get/set a History API state via pushState & replaceState methods. Note that: - Angular treats states undefined and null as the same; trying to change one to the other without touching the URL won't do anything. This is necessary to prevent infinite digest loops when setting the URL to itself in IE<10 in the HTML5 hash fallback mode. - The state() method is not compatible with browsers not supporting the HTML5 History API, e.g. IE 9 or Android < 4.0. Closes angular#9027

mgol · 2014-10-07T19:40:05Z

@IgorMinar @tbosch I merged Igor's changes and rebased over master and now I have one test failure on a test introduced today:

angular.js/test/ng/browserSpecs.js

Line 820 in 8ee1ba4

    
           expect(browser.url).toHaveBeenCalledWith('http://server/#/some/deep/path', false);

Could you have a look?

Adds $location state method allowing to get/set a History API state via pushState & replaceState methods. Note that: - Angular treats states undefined and null as the same; trying to change one to the other without touching the URL won't do anything. This is necessary to prevent infinite digest loops when setting the URL to itself in IE<10 in the HTML5 hash fallback mode. - The state() method is not compatible with browsers not supporting the HTML5 History API, e.g. IE 9 or Android < 4.0. Closes angular#9027

mgol mentioned this pull request Sep 11, 2014

feat($location): add support for state and title in pushState #3325

Closed

mgol force-pushed the pushState branch from 8846f3a to e3d49f6 Compare September 11, 2014 04:32

mgol self-assigned this Sep 11, 2014

mgol added this to the 1.3.0-rc.2 milestone Sep 11, 2014

mgol force-pushed the pushState branch from e3d49f6 to 7e5540f Compare September 11, 2014 05:06

mgol force-pushed the pushState branch from 7e5540f to 53d6e39 Compare September 11, 2014 05:07

mary-poppins added cla: yes and removed cla: no labels Sep 11, 2014

mgol reviewed Sep 11, 2014
View reviewed changes

tbosch modified the milestones: 1.3.0-rc.2, 1.3.0-rc.3 Sep 15, 2014

petebacondarwin force-pushed the master branch from 02dc2aa to fd2d6c0 Compare September 16, 2014 11:26

mgol force-pushed the pushState branch from 53d6e39 to ddf975f Compare September 17, 2014 17:20

mgol force-pushed the pushState branch from ddf975f to 2963115 Compare September 20, 2014 11:27

mgol added this to the 1.3.0 milestone Sep 23, 2014

mgol force-pushed the pushState branch from 2963115 to ba5f4fc Compare September 24, 2014 11:16

IgorMinar reviewed Sep 26, 2014
View reviewed changes

mgol force-pushed the pushState branch from b8eded2 to 82973b7 Compare October 4, 2014 17:25

mgol changed the title ~~feat($location): add support for state and title in pushState~~ feat($location): add support for History API state handling Oct 4, 2014

IgorMinar reviewed Oct 6, 2014
View reviewed changes

btford force-pushed the master branch from 9011a65 to 46db47b Compare October 6, 2014 22:54

mgol force-pushed the pushState branch from 82973b7 to a99c8c6 Compare October 7, 2014 01:57

mgol reviewed Oct 7, 2014
View reviewed changes

IgorMinar reviewed Oct 7, 2014
View reviewed changes

mgol force-pushed the pushState branch from a99c8c6 to 2120627 Compare October 7, 2014 18:52

mgol force-pushed the pushState branch from 2120627 to c0f186d Compare October 7, 2014 19:36

mgol force-pushed the pushState branch from c0f186d to dbaf852 Compare October 7, 2014 19:38

mgol force-pushed the pushState branch from dbaf852 to d7c0ebd Compare October 7, 2014 19:48

IgorMinar mentioned this pull request Oct 7, 2014

feat($location): add support for History API state handling #9476

Closed

mgol closed this in 6fd36de Oct 7, 2014

mgol deleted the pushState branch October 8, 2014 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat($location): add support for History API state handling #9027

feat($location): add support for History API state handling #9027

mgol commented Sep 11, 2014

mgol commented Sep 11, 2014

mgol commented Sep 11, 2014

mgol Sep 11, 2014

IgorMinar Sep 26, 2014

IgorMinar Sep 27, 2014

mgol Sep 30, 2014

mgol Sep 30, 2014

mgol Sep 30, 2014

mgol commented Sep 23, 2014

IgorMinar Sep 26, 2014

mgol commented Oct 4, 2014

mgol commented Oct 6, 2014

IgorMinar Oct 6, 2014

mgol Oct 6, 2014

mgol Oct 6, 2014

IgorMinar Oct 6, 2014

mgol Oct 6, 2014

mgol Oct 7, 2014

mgol commented Oct 7, 2014

mgol commented Oct 7, 2014

IgorMinar Oct 7, 2014

mgol Oct 7, 2014

mgol commented Oct 7, 2014

		var currentReplace = $location.$$replace;
		var currentStateSent = $location.$$stateSent && changeCounter;

feat($location): add support for History API state handling #9027

feat($location): add support for History API state handling #9027

Conversation

mgol commented Sep 11, 2014

mgol commented Sep 11, 2014

mgol commented Sep 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgol commented Sep 23, 2014

Choose a reason for hiding this comment

mgol commented Oct 4, 2014

mgol commented Oct 6, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgol commented Oct 7, 2014

mgol commented Oct 7, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgol commented Oct 7, 2014