Skip to content
This repository has been archived by the owner on Oct 11, 2019. It is now read-only.

Commit

Permalink
fixed 304 Not Modified response parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
@aishek
aishek committed Jan 31, 2014
1 parent 3b86b5c commit 1f219e0
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 16 deletions.
48 changes: 48 additions & 0 deletions lib/assets/javascripts/_dom_parser.js.coffee
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
class DOMParser
parse: (html) ->
@_get_parser()(html)

_get_parser: ->
@_document_parser ?= @_parser_factory()

# fully taken from Turbolinks :)
_parser_factory: ->
createDocumentUsingParser = (html) ->
(new DOMParser).parseFromString html, 'text/html'

createDocumentUsingDOM = (html) ->
doc = document.implementation.createHTMLDocument ''
doc.documentElement.innerHTML = html
doc

createDocumentUsingWrite = (html) ->
doc = document.implementation.createHTMLDocument ''
doc.open 'replace'
doc.write html
doc.close()
doc

# Use createDocumentUsingParser if DOMParser is defined and natively
# supports 'text/html' parsing (Firefox 12+, IE 10)
#
# Use createDocumentUsingDOM if createDocumentUsingParser throws an exception
# due to unsupported type 'text/html' (Firefox < 12, Opera)
#
# Use createDocumentUsingWrite if:
# - DOMParser isn't defined
# - createDocumentUsingParser returns null due to unsupported type 'text/html' (Chrome, Safari)
# - createDocumentUsingDOM doesn't create a valid HTML document (safeguarding against potential edge cases)
try
if window.DOMParser
testDoc = createDocumentUsingParser '<html><body><p>test'
createDocumentUsingParser
catch e
testDoc = createDocumentUsingDOM '<html><body><p>test'
createDocumentUsingDOM
finally
unless testDoc?.body?.childNodes.length is 1
return createDocumentUsingWrite


window._Wiselinks ?= {}
window._Wiselinks.DOMParser = DOMParser
37 changes: 21 additions & 16 deletions lib/assets/javascripts/_request_manager.js.coffee
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#= require _response

class RequestManager
constructor: (@options = {}) ->

Expand Down Expand Up @@ -25,22 +27,7 @@ class RequestManager
dataType: "html"
).done(
(data, status, xhr) ->
url = self._normalize(xhr.getResponseHeader('X-Wiselinks-Url'))
assets_digest = xhr.getResponseHeader('X-Wiselinks-Assets-Digest')

if self._assets_changed(assets_digest)
window.location.reload(true)
else
state = History.getState()
if url? && (url != self._normalize(state.url))
self._redirect_to(url, $target, state, xhr)

$target.html(data).promise().done(
->
self._title(xhr.getResponseHeader('X-Wiselinks-Title'))
self._done($target, status, state, data)
)

self._html_loaded($target, data, status, xhr)
).fail(
(xhr, status, error) ->
self._fail($target, status, state, error, xhr.status)
Expand Down Expand Up @@ -77,6 +64,24 @@ class RequestManager
[$target, status, decodeURI(state.url), data]
)

_html_loaded: ($target, data, status, xhr) ->
url = @_normalize(xhr.getResponseHeader('X-Wiselinks-Url'))
assets_digest = xhr.getResponseHeader('X-Wiselinks-Assets-Digest')

if @_assets_changed(assets_digest)
window.location.reload(true)
else
state = History.getState()
if url? && (url != @_normalize(state.url))
@_redirect_to(url, $target, state, xhr)

response = new window._Wiselinks.Response(data, xhr, $target)
$target.html(response.content()).promise().done(
=>
@_title(response.title())
@_done($target, status, state, response.content())
)

_fail: ($target, status, state, error, code) ->
$(document).trigger('page:fail'
[$target, status, decodeURI(state.url), error, code]
Expand Down
45 changes: 45 additions & 0 deletions lib/assets/javascripts/_response.js.coffee
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#= require _dom_parser

# if server responds with 304, sometimes you may get
# full page source instead of just wiselinks part, so
# you need to use different content extract strategies
class Response
@_document_parser: null

@_get_document_parser: ->
Response._document_parser ?= new window._Wiselinks.DOMParser


constructor: (@html, @xhr, @$target) ->

content: ->
@_content ?= @_extract_content()

title: ->
@_title ?= @_extract_title()

_extract_title: ->
if @_is_full_document_response()
$('title', @_get_doc()).text()
else
@xhr.getResponseHeader('X-Wiselinks-Title')

_extract_content: ->
if @_is_full_document_response()
@_get_doc_target_node().html()
else
@html

_is_full_document_response: ->
@_get_doc_target_node().length is 1

_get_doc_target_node: ->
@$doc_target_node ?= $(@$target.selector, @_get_doc())

_get_doc: ->
@_doc ?= Response._get_document_parser().parse(@html)



window._Wiselinks ?= {}
window._Wiselinks.Response = Response

7 comments on commit 1f219e0

@amnesia7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aishek @igor-alexandrov , this commit seems to have broken compatibility with IE8.

The comment in the parser code says "fully taken from Turbolinks" but turbolinks doesn't support IE8 except via the fallback whereas wiselinks did until this change was implemented #76

Any ideas how it can be fixed to allow it to work for IE8 again?

Not sure if it helps or not but I found a similiar issue here Notalib/LYT#313 and they resolved it with this workaround mzedeler/LYT@d55850f

@aishek
Copy link
Contributor Author

@aishek aishek commented on 1f219e0 Jul 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amnesia7 thanks for note. I'll check this out soon.

@aishek
Copy link
Contributor Author

@aishek aishek commented on 1f219e0 Jul 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amnesia7 fixed in #83, please test in your case

@amnesia7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I doing something wrong. I'm trying to use:

gem 'wiselinks', github: 'aishek/wiselinks', branch: 'ie8-fallback-fix'

but it keeps saying:

wiselinks at /....../bundler/gems/wiselinks-ba85c7202d2e did not have a valid gemspec.
This prevents bundler from installing bins or native extensions, but that may not affect its functionality.
The validation message from Rubygems was:
  duplicate dependency on rspec (>= 0, development), (>= 0) use:
    add_runtime_dependency 'rspec', '>= 0', '>= 0'
Using wiselinks 1.2.2 (was 1.2.1) from git://github.com/aishek/wiselinks.git (at ie8-fallback-fix)

which sounds like it could be ok but after a bundle clean --force to remove the existing wiselinks so the page doesn't try to use the wrong one (Removing wiselinks (1.2.1)) my page then says "couldn't find file 'wiselinks'"

@amnesia7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried restarting dev server and now I get:

SCRIPT5022: History.js Adapter has already been loaded...

using:

window.wiselinks = new Wiselinks(pageContainer, { html4_root_path: '/projects' });

Any ideas what I'm doing wrong @aishek ?

@aishek
Copy link
Contributor Author

@aishek aishek commented on 1f219e0 Jul 29, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amnesia7 keep calm and do more research :) @igor-alexandrov has precompiled assets in dummy app – remove them and you'll be able to test changes

@amnesia7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aishek your change looks to work fine.

My issue lies in the fact that when I use the back button it is filling my pageContainer with the full html template which includes loading the history.js file (as well as everything else) again.

I have only applied the following in my projects#index for wiselinks and don't have my own wiselinks layout file:

    if request.wiselinks_partial?
      render :partial => 'list_results'
    end

I haven't used request.wiselinks_template? which I think gets called when you use the back button and get back to the first wiselinks-enabled page that was loaded but I haven't needed to with html5 browsers. They work fine with just the partial statement. Any ideas?

Please sign in to comment.