Permalink
Browse files

Fix for issue 2406 - Navigation from one page back to multi-page temp…

…late

- Make sure that our hashchange resolves non-path hashes against the documentBase. This prevents the resulting changePath() call from incorrectly resolving against the URL for the current active (external) page.

- Fixed a problem in the push-state code. A hashchange event is *NOT* fired when navigating back (window.history.back()) from an external page to an internal page. This makes sense when you think about it since hashchange is only ever fired when the hash of the current document url changes, not when the document url itself changes. The fix was to make sure that the pushstate hashchange callback always sets a state object, even on embedded page URLs. This allows the hashchange callback to be triggered from within onPopState().
  • Loading branch information...
1 parent 824bd12 commit 1a92be0240f106c7cb3802edff97ddb5bf09f326 @jblas jblas committed Sep 13, 2011
Showing with 57 additions and 12 deletions.
  1. +6 −1 js/jquery.mobile.navigation.js
  2. +21 −11 js/jquery.mobile.navigation.pushstate.js
  3. +30 −0 tests/unit/navigation/navigation_core.js
@@ -1351,7 +1351,12 @@
//if to is defined, load it
if ( to ) {
- to = ( typeof to === "string" && !path.isPath( to ) ) ? ( '#' + to ) : to;
+ // At this point, 'to' can be one of 3 things, a cached page element from
+ // a history stack entry, an id, or site-relative/absolute URL. If 'to' is
+ // an id, we need to resolve it against the documentBase, not the location.href,
+ // since the hashchange could've been the result of a forward/backward navigation
+ // that crosses from an external page/dialog to an internal page/dialog.
+ to = ( typeof to === "string" && !path.isPath( to ) ) ? ( path.makeUrlAbsolute( '#' + to, documentBase ) ) : to;
$.mobile.changePage( to, changePageOptions );
} else {
//there's no hash, go to the first page in the dom
@@ -52,24 +52,34 @@
// NOTE this takes place *after* the vanilla navigation hash change
// handling has taken place and set the state of the DOM
onHashChange: function( e ) {
- var href, state;
+ var href, state,
+ hash = location.hash,
+ isPath = $.mobile.path.isPath( hash );
+ hash = isPath ? hash.replace( "#", "" ) : hash;
self.hashchangeFired = true;
- // only replaceState when the hash doesn't represent an embeded page
- if( $.mobile.path.isPath(location.hash) ) {
+ // propulate the hash when its not available
+ state = self.state();
- // propulate the hash when its not available
- state = self.state();
-
- // make the hash abolute with the current href
- href = $.mobile.path.makeUrlAbsolute( state.hash.replace("#", ""), location.href );
+ // make the hash abolute with the current href
+ href = $.mobile.path.makeUrlAbsolute( hash, location.href );
+ if ( isPath ) {
href = self.resetUIKeys( href );
-
- // replace the current url with the new href and store the state
- history.replaceState( state, document.title, href );
}
+
+ // replace the current url with the new href and store the state
+ // Note that in some cases we might be replacing an url with the
+ // same url. We do this anyways because we need to make sure that
+ // all of our history entries have a state object associated with
+ // them. This allows us to work around the case where window.history.back()
+ // is called to transition from an external page to an embedded page.
+ // In that particular case, a hashchange event is *NOT* generated by the browser.
+ // Ensuring each history entry has a state object means that onPopState()
+ // will always trigger our hashchange callback even when a hashchange event
+ // is not fired.
+ history.replaceState( state, document.title, href );
},
// on popstate (ie back or forward) we need to replace the hash that was there previously
@@ -43,6 +43,36 @@
}
});
+ asyncTest( "window.history.back() from external to internal page", function(){
+
+ $.testHelper.pageSequence([
+
+ // open our test page
+ function(){
+ $.testHelper.openPage("#active-state-page1");
+ },
+
+ function(){
+ ok( $.mobile.activePage[0] === $( "#active-state-page1" )[ 0 ], "successful navigation to internal page." );
+
+ //location.hash = siteDirectory + "external.html";
+ $.mobile.changePage("external.html");
+ },
+
+ function(){
+ ok( $.mobile.activePage[0] !== $( "#active-state-page1" )[ 0 ], "successful navigation to external page." );
+
+ window.history.back();
+ },
+
+ function(){
+ ok( $.mobile.activePage[0] === $( "#active-state-page1" )[ 0 ], "successful navigation back to internal page." );
+
+ start();
+ }
+ ]);
+ });
+
asyncTest( "external page is removed from the DOM after pagehide", function(){
$.testHelper.pageSequence([
navigateTestRoot,

1 comment on commit 1a92be0

@johnbender
Contributor

Super duper nitpicky, but it might be good to separate out the test commits from the code changes. I almost always push two commits at a time.

Please sign in to comment.