Merge branch 'scroll-fixes'

Closes #474

Author: mislav

jquery.pjax.js

@@ -225,7 +225,9 @@ function pjax(options) {
       settings.timeout = 0
     }
 
-    options.requestUrl = stripInternalParams(parseURL(settings.url).href)
+    var url = parseURL(settings.url)
+    url.hash = hash
+    options.requestUrl = stripInternalParams(url.href)
   }
 
   options.complete = function(xhr, textStatus) {
@@ -259,6 +261,12 @@ function pjax(options) {
 
     var container = extractContainer(data, xhr, options)
 
+    var url = parseURL(container.url)
+    if (hash) {
+      url.hash = hash
+      container.url = url.href
+    }
+
     // If there is a layout version mismatch, hard load the new url
     if (currentVersion && latestVersion && currentVersion !== latestVersion) {
       locationReplace(container.url)
@@ -309,28 +317,17 @@ function pjax(options) {
 
     executeScriptTags(container.scripts)
 
-    // Scroll to top by default
-    if (typeof options.scrollTo === 'number')
-      $(window).scrollTop(options.scrollTo)
-
-    // If the URL has a hash in it, make sure the browser
-    // knows to navigate to the hash.
-    if ( hash !== '' ) {
-      // Avoid using simple hash set here. Will add another history
-      // entry. Replace the url with replaceState and scroll to target
-      // by hand.
-      //
-      //   window.location.hash = hash
-      var url = parseURL(container.url)
-      url.hash = hash
-
-      pjax.state.url = url.href
-      window.history.replaceState(pjax.state, container.title, url.href)
+    var scrollTo = options.scrollTo
 
-      var target = document.getElementById(url.hash.slice(1))
-      if (target) $(window).scrollTop($(target).offset().top)
+    // Ensure browser scrolls to the element referenced by the URL anchor
+    if (hash) {
+      var name = decodeURIComponent(hash.slice(1))
+      var target = document.getElementById(name) || document.getElementsByName(name)[0]
+      if (target) scrollTo = $(target).offset().top
     }
 
+    if (typeof scrollTo == 'number') $(window).scrollTop(scrollTo)
+
     fire('pjax:success', [data, status, xhr, options])
   }
 
@@ -566,8 +563,8 @@ function cloneContents(container) {
 // Returns String.
 function stripParam(url, name) {
   return url
-    .replace(new RegExp('[?&]' + name + '=[^&]*'), '')
-    .replace(/[?&]$/, '')
+    .replace(new RegExp('[?&]' + name + '=[^&#]*'), '')
+    .replace(/[?&]($|#)/, '\1')
     .replace(/[?&]/, '?')
 }
 

test/app.rb

@@ -21,7 +21,7 @@ def title(str)
 
 after do
   if pjax?
-    response.headers['X-PJAX-URL'] = request.url
+    response.headers['X-PJAX-URL'] ||= request.url
     response.headers['X-PJAX-Version'] = 'v1'
   end
 end
@@ -48,7 +48,17 @@ def title(str)
 end
 
 get '/redirect.html' do
-  redirect "/hello.html"
+  if params[:anchor]
+    path = "/hello.html##{params[:anchor]}"
+    if pjax?
+      response.headers['X-PJAX-URL'] = uri(path)
+      status 200
+    else
+      redirect path
+    end
+  else
+    redirect "/hello.html"
+  end
 end
 
 get '/timeout.html' do

test/unit/fn_pjax.js

@@ -230,21 +230,6 @@ if ($.support.pjax) {
     frame.$("a[href='/dinosaurs.html']").click()
   })
 
-
-  asyncTest("scrolls to anchor after load", function() {
-    var frame = this.frame
-
-    frame.$("#main").pjax("a").on("pjax:end", function() {
-      equal(frame.location.pathname, "/dinosaurs.html")
-      equal(frame.location.hash, "#main")
-      start()
-    })
-
-    var link = frame.$("a[href='/dinosaurs.html']")
-    link.attr('href', "/dinosaurs.html#main")
-    link.click()
-  })
-
   asyncTest("triggers pjax:click event from link", function() {
     var frame = this.frame
 

test/unit/pjax.js

@@ -1192,24 +1192,10 @@ if ($.support.pjax) {
     })
   })
 
-  asyncTest("follows redirect with X-PJAX-URL header", function() {
-    var frame = this.frame
-
-    frame.$('#main').on("pjax:success", function() {
-      equal(frame.location.pathname, "/hello.html")
-      equal(frame.$("#main > p").html().trim(), "Hello!")
-      start()
-    })
-    frame.$.pjax({
-      url: "redirect.html",
-      container: "#main"
-    })
-  })
-
   asyncTest("lazily sets initial $.pjax.state", function() {
     var frame = this.frame
 
-    ok(!frame.$.pjax.state)
+    equal(frame.$.pjax.state, null)
 
     frame.$('#main').on("pjax:success", function() {
       start()
@@ -1219,24 +1205,28 @@ if ($.support.pjax) {
       container: "#main"
     })
 
-    ok(frame.$.pjax.state.id)
-    ok(frame.$.pjax.state.url.match("/home.html"))
-    equal(frame.$.pjax.state.container, "#main")
+    var initialState = frame.$.pjax.state
+    ok(initialState.id)
+    equal(initialState.url, "http://" + frame.location.host + "/home.html")
+    equal(initialState.container, "#main")
   })
 
   asyncTest("updates $.pjax.state to new page", function() {
     var frame = this.frame
 
     frame.$('#main').on("pjax:success", function() {
-      ok(frame.$.pjax.state.id)
-      ok(frame.$.pjax.state.url.match("/hello.html"))
-      equal(frame.$.pjax.state.container, "#main")
+      var state = frame.$.pjax.state
+      ok(state.id)
+      equal(state.url, "http://" + frame.location.host + "/hello.html#new")
+      equal(state.container, "#main")
       start()
     })
     frame.$.pjax({
-      url: "hello.html",
+      url: "hello.html#new",
       container: "#main"
     })
+
+    var initialState = frame.$.pjax.state
   })
 
   asyncTest("new id is generated for new pages", function() {

test/unit/pjax_fallback.js

@@ -5,6 +5,10 @@ $.each([true, false], function() {
 var disabled = this == false
 var s = disabled ? " (disabled)" : ""
 
+var ua = navigator.userAgent
+var safari = ua.match("Safari") && !ua.match("Chrome") && !ua.match("Edge")
+var chrome = ua.match("Chrome") && !ua.match("Edge")
+
 module("$.pjax fallback"+s, {
   setup: function() {
     var self = this
@@ -123,6 +127,8 @@ asyncTest("scrolls to anchor at top page"+s, function() {
 
   this.loaded = function(frame) {
     setTimeout(function() {
+      equal(frame.location.pathname, "/anchor.html")
+      equal(frame.location.hash, "#top")
       equal(frame.window.scrollY, 8)
       start()
     }, 100)
@@ -132,6 +138,14 @@ asyncTest("scrolls to anchor at top page"+s, function() {
     url: "/anchor.html#top",
     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")
+  }
 })
 
 asyncTest("empty anchor doesn't scroll page"+s, function() {
@@ -170,7 +184,23 @@ asyncTest("scrolls to anchor at bottom page"+s, function() {
   })
 })
 
+asyncTest("scrolls to named encoded anchor"+s, function() {
+  var frame = this.frame
 
+  equal(frame.window.scrollY, 0)
+
+  this.loaded = function(frame) {
+    setTimeout(function() {
+      equal(frame.window.scrollY, 10008)
+      start()
+    }, 10)
+  }
+
+  frame.$.pjax({
+    url: "/anchor.html#%62%6F%74%74%6F%6D",
+    container: "#main"
+  })
+})
 
 asyncTest("sets GET method"+s, function() {
   var frame = this.frame
@@ -396,4 +426,36 @@ asyncTest("handle form submit"+s, function() {
   frame.$("form").submit()
 })
 
+asyncTest("browser URL is correct after redirect"+s, function() {
+  var frame = this.frame
+
+  this.loaded = function() {
+    equal(frame.location.pathname, "/hello.html")
+    var expectedHash = safari && disabled ? "" : "#new"
+    equal(frame.location.hash, expectedHash)
+    start()
+  }
+
+  frame.$.pjax({
+    url: "redirect.html#new",
+    container: "#main"
+  })
+})
+
+asyncTest("server can't affect anchor after redirect"+s, function() {
+  var frame = this.frame
+
+  this.loaded = function() {
+    equal(frame.location.pathname, "/hello.html")
+    var expectedHash = safari && disabled ? "" : "#new"
+    equal(frame.location.hash, expectedHash)
+    start()
+  }
+
+  frame.$.pjax({
+    url: "redirect.html?anchor=server#new",
+    container: "#main"
+  })
+})
+
 })

test/views/anchor.erb

@@ -1,4 +1,4 @@
 <%= title 'Anchor' %>
 <div id="top" style="width:500px;height:10000px;"></div>
-<div id="bottom" style="width:500px;height:10000px;"></div>
+<a name="bottom" style="width:500px;height:10000px;display:block;"></a>
 <script type="text/javascript">window.parent.iframeLoad(window)</script>