Delay calling pushState() until the xhr succeeds

jquery.pjax.js

@@ -244,7 +244,7 @@ function pjax(options) {
 
     var allowed = fire('pjax:error', [xhr, textStatus, errorThrown, options])
     if (options.type == 'GET' && textStatus !== 'abort' && allowed) {
-      locationReplace(container.url)
+      window.location.assign(container.url)
     }
   }
 
@@ -269,16 +269,20 @@ function pjax(options) {
 
     // If there is a layout version mismatch, hard load the new url
     if (currentVersion && latestVersion && currentVersion !== latestVersion) {
-      locationReplace(container.url)
+      window.location.assign(container.url)
       return
     }
 
     // If the new response is missing a body, hard load the page
     if (!container.contents) {
-      locationReplace(container.url)
+      window.location.assign(container.url)
       return
     }
 
+    if (options.push) {
+      cachePush(pjax.state.id, cloneContents(context))
+    }
+
     pjax.state = {
       id: options.id || uniqueId(),
       url: container.url,
@@ -288,7 +292,9 @@ function pjax(options) {
       timeout: options.timeout
     }
 
-    if (options.push || options.replace) {
+    if (options.push) {
+      window.history.pushState(pjax.state, container.title, container.url)
+    } else if (options.replace) {
       window.history.replaceState(pjax.state, container.title, container.url)
     }
 
@@ -355,13 +361,6 @@ function pjax(options) {
   var xhr = pjax.xhr = $.ajax(options)
 
   if (xhr.readyState > 0) {
-    if (options.push && !options.replace) {
-      // Cache current container element before replacing it
-      cachePush(pjax.state.id, cloneContents(context))
-
-      window.history.pushState(null, "", options.requestUrl)
-    }
-
     fire('pjax:start', [xhr, options])
     fire('pjax:send', [xhr, options])
   }
@@ -447,12 +446,6 @@ function onPjaxPopstate(event) {
     if (container.length) {
       var contents = cacheMapping[state.id]
 
-      if (previousState) {
-        // Cache current container before replacement and inform the
-        // cache which direction the history shifted.
-        cachePop(direction, previousState.id, cloneContents(container))
-      }
-
       var popstateEvent = $.Event('pjax:popstate', {
         state: state,
         direction: direction
@@ -469,9 +462,19 @@ function onPjaxPopstate(event) {
         scrollTo: false
       }
 
+      function doCachePop() {
+        if (previousState) {
+          cachePop(direction, previousState.id, cloneContents(container))
+        }
+      }
+
       if (contents) {
         container.trigger('pjax:start', [null, options])
 
+        // Only touch the cache after pjax:start for consistency with the
+        // sequence of events in pjax().
+        doCachePop()
+
         pjax.state = state
         if (state.title) document.title = state.title
         var beforeReplaceEvent = $.Event('pjax:beforeReplace', {
@@ -483,6 +486,7 @@ function onPjaxPopstate(event) {
 
         container.trigger('pjax:end', [null, options])
       } else {
+        doCachePop()
         pjax(options)
       }
 

test/app.rb

@@ -90,6 +90,15 @@ def title(str)
   erb :boom_sans_pjax, :layout => false
 end
 
+get '/referer_timeout.html' do
+  if pjax?
+    sleep 1
+    erb :referer, :layout => false
+  else
+    erb :referer
+  end
+end
+
 get '/:page.html' do
   erb :"#{params[:page]}", :layout => !pjax?
 end

test/unit/pjax.js

@@ -322,10 +322,6 @@ if ($.support.pjax) {
       data: { foo: 1, bar: 2 },
       container: "#main"
     })
-
-    // URL is set immediately
-    equal(frame.location.pathname, "/env.html")
-    equal(frame.location.search, "?foo=1&bar=2")
   })
 
   asyncTest("GET data is merged into query string", function() {
@@ -345,10 +341,6 @@ if ($.support.pjax) {
       data: { bar: 2 },
       container: "#main"
     })
-
-    // URL is set immediately
-    equal(frame.location.pathname, "/env.html")
-    equal(frame.location.search, "?foo=1&bar=2")
   })
 
   asyncTest("mixed containers", function() {
@@ -745,9 +737,6 @@ if ($.support.pjax) {
       container: "#main"
     })
 
-    equal(frame.location.pathname, "/timeout.html")
-    equal(frame.location.hash, "#hello")
-
     this.iframe.onload = function() {
       equal(frame.$("#main p").html(), "SLOW DOWN!")
       equal(frame.location.pathname, "/timeout.html")
@@ -817,7 +806,7 @@ if ($.support.pjax) {
     var frame = this.frame
 
     frame.$("#main").on("pjax:complete", function() {
-      equal(frame.location.pathname, "/boom.html")
+      equal(frame.location.pathname, "/home.html")
       start()
     })
     frame.$("#main").on("pjax:error", function(event, xhr) {
@@ -835,6 +824,26 @@ if ($.support.pjax) {
     })
   })
 
+  asyncTest("address bar only updates on success", function() {
+    var frame = this.frame
+
+    equal(frame.location.pathname, "/home.html")
+
+    frame.$("#main").on('pjax:send', function() {
+      equal(frame.location.pathname, "/home.html")
+    })
+
+    frame.$("#main").on('pjax:end', function() {
+      equal(frame.location.pathname, "/hello.html")
+      start()
+    })
+
+    frame.$.pjax({
+      url: "hello.html",
+      container: "#main"
+    })
+  })
+
   function goBack(frame, callback) {
     setTimeout(function() {
       frame.$("#main").one("pjax:end", callback)
@@ -852,39 +861,102 @@ if ($.support.pjax) {
   asyncTest("clicking back while loading cancels XHR", function() {
     var frame = this.frame
 
-    frame.$('#main').on('pjax:timeout', function(event) {
-      event.preventDefault()
+    equal(frame.location.pathname, "/home.html")
+    equal(frame.document.title, "Home")
+
+    frame.$("#main").on('pjax:timeout', function(e) {
+      e.preventDefault();
     })
 
-    frame.$("#main").one('pjax:send', function() {
+    frame.$("#main").one('pjax:complete', function() {
+
+      equal(frame.location.pathname, "/hello.html")
+      equal(frame.document.title, "Hello")
+
+      frame.$("#main").one('pjax:send', function() {
 
-      // Check that our request is aborted (need to check
-      // how robust this is across browsers)
-      frame.$("#main").one('pjax:complete', function(e, xhr, textStatus) {
-        equal(xhr.status, 0)
-        equal(textStatus, 'abort')
+        // don't use goBack here, because pjax:end isn't triggered
+        // when clicking back while loading
+
+        frame.$("#main").one('pjax:complete', function(e, xhr, textStatus) {
+          equal(xhr.status, 0);
+          equal(textStatus, 'abort')
+        })
+
+        frame.history.back();
+
+        // Make sure the URL and content remain the same after the
+        // XHR would have arrived (delay on timeout.html is 1s)
+        setTimeout(function() {
+          var afterBackLocation = frame.location.pathname
+          var afterBackTitle = frame.document.title
+
+          setTimeout(function() {
+            equal(frame.location.pathname, afterBackLocation)
+            equal(frame.document.title, afterBackTitle)
+            start()
+          }, 1000)
+        }, 500)
       })
 
-      setTimeout(function() {
-        frame.history.back()
-      }, 250)
+      frame.$.pjax({
+        url: "timeout.html",
+        container: "#main"
+      })
+    })
 
-      // Make sure the URL and content remain the same after the
-      // XHR would have arrived (delay on timeout.html is 1s)
-      setTimeout(function() {
-        var afterBackLocation = frame.location.pathname
-        var afterBackTitle = frame.document.title
+    frame.$.pjax({
+      url: "hello.html",
+      container: "#main"
+    })
+
+  })
+
+  asyncTest("clicking back while loading maintains history", function() {
+    var frame = this.frame
+
+    equal(frame.location.pathname, "/home.html")
+    equal(frame.document.title, "Home")
+
+    frame.$("#main").on('pjax:timeout', function(e) {
+      e.preventDefault();
+    })
+
+    frame.$("#main").one('pjax:complete', function() {
+
+      equal(frame.location.pathname, "/hello.html")
+      equal(frame.document.title, "Hello")
+
+      frame.$("#main").one('pjax:send', function() {
+
+        // don't use goBack here, because pjax:end isn't triggered
+        // when clicking back while loading
+
+        frame.history.back();
 
         setTimeout(function() {
-          equal(frame.location.pathname, afterBackLocation)
-          equal(frame.document.title, afterBackTitle)
-          start()
-        }, 1000)
-      }, 500)
+          equal(frame.location.pathname, "/home.html")
+          equal(frame.document.title, "Home")
+
+          frame.history.forward()
+
+          setTimeout(function() {
+            equal(frame.location.pathname, "/hello.html")
+            equal(frame.document.title, "Hello")
+            start()
+          }, 250)
+
+        }, 1500)
+      })
+
+      frame.$.pjax({
+        url: "timeout.html",
+        container: "#main"
+      })
     })
 
     frame.$.pjax({
-      url: "timeout.html",
+      url: "hello.html",
       container: "#main"
     })
   })

test/unit/pjax_fallback.js

@@ -103,6 +103,49 @@ asyncTest("sends correct HTTP referer"+s, function() {
   })
 })
 
+asyncTest("sends correct HTTP referer after failed request"+s, function() {
+  var frame = this.frame
+
+  $('iframe')[0].onload = function() {
+    var referer = frame.document.getElementById("referer").textContent
+    ok(referer.match("/home.html"), referer)
+    start()
+  }
+
+  frame.$.pjax({
+    url: "/referer_timeout.html",
+    container: "#main"
+  })
+})
+
+asyncTest("adds entry to browser history"+s, function() {
+  var frame = this.frame
+  var count = 0
+
+  frame.onpopstate = function() {
+    window.iframeLoad(frame)
+  }
+
+  this.loaded = function() {
+    count++
+
+    if (count == 1) {
+      equal(frame.location.pathname, "/hello.html")
+      ok(frame.history.length > 1)
+      frame.history.back()
+    } else if (count == 2) {
+      equal(frame.location.pathname, "/home.html")
+      frame.history.forward()
+      start()
+    }
+  }
+
+  frame.$.pjax({
+    url: "hello.html",
+    container: "#main"
+  })
+})
+
 asyncTest("scrolls to top of the page"+s, function() {
   var frame = this.frame
 
@@ -139,13 +182,8 @@ asyncTest("scrolls to anchor at top page"+s, function() {
     container: "#main"
   })
 
-  if (disabled) {
-    equal(frame.location.pathname, "/home.html")
-    equal(frame.location.hash, "")
-  } else {
-    equal(frame.location.pathname, "/anchor.html")
-    equal(frame.location.hash, "#top")
-  }
+  equal(frame.location.pathname, "/home.html")
+  equal(frame.location.hash, "")
 })
 
 asyncTest("empty anchor doesn't scroll page"+s, function() {