Browse Source

Fix root redirects stripping the #fragment from the original URL

Thibaut 10 years ago
parent
commit
217d5016d0
3 changed files with 47 additions and 14 deletions
  1. 11 4
      assets/javascripts/app/router.coffee
  2. 17 4
      lib/app.rb
  3. 19 6
      test/app_test.rb

+ 11 - 4
assets/javascripts/app/router.coffee

@@ -107,16 +107,23 @@ class app.Router
     if (path = location.pathname.replace /^\/{2,}/g, '/') isnt location.pathname
     if (path = location.pathname.replace /^\/{2,}/g, '/') isnt location.pathname
       page.replace path + location.search + location.hash, null, true
       page.replace path + location.search + location.hash, null, true
 
 
-    # When the path is "/#/path", replace it with "/path"
-    if @isRoot() and path = @getInitialPath()
-      page.replace path + location.search, null, true
+    if @isRoot()
+      if path = @getInitialPathFromHash()
+        page.replace path + location.search, null, true
+      else if path = @getInitialPathFromCookie()
+        page.replace path + location.search + location.hash, null, true
     return
     return
 
 
-  getInitialPath: ->
+  getInitialPathFromHash: ->
     try
     try
       (new RegExp "#/(.+)").exec(decodeURIComponent location.hash)?[1]
       (new RegExp "#/(.+)").exec(decodeURIComponent location.hash)?[1]
     catch
     catch
 
 
+  getInitialPathFromCookie: ->
+    if path = Cookies.get('initial_path')
+      Cookies.expire('initial_path')
+      path
+
   replaceHash: (hash) ->
   replaceHash: (hash) ->
     page.replace location.pathname + location.search + (hash or ''), null, true
     page.replace location.pathname + location.search + (hash or ''), null, true
     return
     return

+ 17 - 4
lib/app.rb

@@ -157,6 +157,15 @@ class App < Sinatra::Application
     def dark_theme?
     def dark_theme?
       app_theme == 'dark'
       app_theme == 'dark'
     end
     end
+
+    def redirect_via_js(path) # courtesy of HTML5 App Cache
+      response.set_cookie :initial_path, value: path, expires: Time.now + 15, path: '/'
+      redirect '/', 302
+    end
+
+    def supports_js_redirection?
+      browser.modern? && !cookies.empty?
+    end
   end
   end
 
 
   before do
   before do
@@ -170,13 +179,17 @@ class App < Sinatra::Application
   end
   end
 
 
   get '/' do
   get '/' do
-    return redirect '/' unless request.query_string.empty?
+    return redirect '/' unless request.query_string.empty? # courtesy of HTML5 App Cache
     erb :index
     erb :index
   end
   end
 
 
   %w(offline about news help).each do |page|
   %w(offline about news help).each do |page|
     get "/#{page}" do
     get "/#{page}" do
-      redirect "/#/#{page}", 302
+      if supports_js_redirection?
+        redirect_via_js "/#{page}"
+      else
+        redirect "/#/#{page}", 302
+      end
     end
     end
   end
   end
 
 
@@ -228,8 +241,8 @@ class App < Sinatra::Application
       redirect "/#{doc}#{type}/#{query_string_for_redirection}"
       redirect "/#{doc}#{type}/#{query_string_for_redirection}"
     elsif rest.length > 1 && rest.end_with?('/')
     elsif rest.length > 1 && rest.end_with?('/')
       redirect "/#{doc}#{type}#{rest[0...-1]}#{query_string_for_redirection}"
       redirect "/#{doc}#{type}#{rest[0...-1]}#{query_string_for_redirection}"
-    elsif docs.include?(doc) && browser.modern?
-      redirect "/##{request.path}", 302
+    elsif docs.include?(doc) && supports_js_redirection?
+      redirect_via_js(request.path)
     else
     else
       erb :other
       erb :other
     end
     end

+ 19 - 6
test/app_test.rb

@@ -5,6 +5,8 @@ require 'app'
 class AppTest < MiniTest::Spec
 class AppTest < MiniTest::Spec
   include Rack::Test::Methods
   include Rack::Test::Methods
 
 
+  MODERN_BROWSER = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'
+
   def app
   def app
     App
     App
   end
   end
@@ -40,13 +42,23 @@ class AppTest < MiniTest::Spec
   end
   end
 
 
   describe "/[static-page]" do
   describe "/[static-page]" do
-    it "redirects to /#/[static-page]" do
+    it "redirects to /#/[static-page] by default" do
       %w(offline about news help).each do |page|
       %w(offline about news help).each do |page|
-        get "/#{page}"
+        get "/#{page}", {}, 'HTTP_USER_AGENT' => MODERN_BROWSER
         assert last_response.redirect?
         assert last_response.redirect?
         assert_equal "http://example.org/#/#{page}", last_response['Location']
         assert_equal "http://example.org/#/#{page}", last_response['Location']
       end
       end
     end
     end
+
+    it "redirects via JS cookie when a cookie exists" do
+      %w(offline about news help).each do |page|
+        set_cookie('foo=bar')
+        get "/#{page}", {}, 'HTTP_USER_AGENT' => MODERN_BROWSER
+        assert last_response.redirect?
+        assert_equal 'http://example.org/', last_response['Location']
+        assert last_response['Set-Cookie'].start_with?("initial_path=%2F#{page}; path=/; expires=")
+      end
+    end
   end
   end
 
 
   describe "/search" do
   describe "/search" do
@@ -109,15 +121,16 @@ class AppTest < MiniTest::Spec
   describe "/[doc]" do
   describe "/[doc]" do
     it "renders when the doc exists and isn't enabled" do
     it "renders when the doc exists and isn't enabled" do
       set_cookie('docs=css')
       set_cookie('docs=css')
-      get '/html/', {}, 'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'
+      get '/html/', {}, 'HTTP_USER_AGENT' => MODERN_BROWSER
       assert last_response.ok?
       assert last_response.ok?
     end
     end
 
 
-    it "redirects to root when the doc exists and is enabled" do
+    it "redirects via JS cookie when the doc exists and is enabled" do
       set_cookie('docs=html')
       set_cookie('docs=html')
-      get '/html/', {}, 'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:39.0) Gecko/20100101 Firefox/39.0'
+      get '/html/', {}, 'HTTP_USER_AGENT' => MODERN_BROWSER
       assert last_response.redirect?
       assert last_response.redirect?
-      assert_equal 'http://example.org/#/html/', last_response['Location']
+      assert_equal 'http://example.org/', last_response['Location']
+      assert last_response['Set-Cookie'].start_with?("initial_path=%2Fhtml%2F; path=/; expires=")
     end
     end
 
 
     it "renders when the doc exists and is enabled, and the request is from Googlebot" do
     it "renders when the doc exists and is enabled, and the request is from Googlebot" do