Skip to content

Commit

Permalink
BUG Fix incorrect parsing of HTML content
Browse files Browse the repository at this point in the history
  • Loading branch information
Damian Mooyman committed Aug 12, 2014
1 parent a369094 commit 98907fb
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions admin/javascript/LeftAndMain.js
Expand Up @@ -585,11 +585,14 @@ jQuery.noConflict();

var newFragments = {}, newContentEls;
// If content type is text/json (ignoring charset and other parameters)
if(xhr.getResponseHeader('Content-Type').match(/^text\/json[ \t]*;?/i)) {
if(xhr.getResponseHeader('Content-Type').match(/^((text)|(application))\/json[ \t]*;?/i)) {
newFragments = data;
} else {

// Fall back to replacing the content fragment if HTML is returned
$data = $(data);
var fragment = document.createDocumentFragment();
jQuery.clean( [ data ], document, fragment, [] );

This comment has been minimized.

Copy link
@dhensby

dhensby Aug 13, 2014

Contributor

This is a bad idea considering .clean was removed in jQuery 1.9 and there's talk of upgrading core jQuery.

.clean is non-documented and non-public API so shouldn't be used.

you'd want .parseHTML instead

This comment has been minimized.

Copy link
@tractorcow

tractorcow Aug 13, 2014

Contributor

Thanks @dhensby :) we are using jquery 1.7 in core so unfortunately we can't rely on api introduced into 1.8.

We can flag this as a part of our "upgrade to 1.9/2.x" solution.

This comment has been minimized.

Copy link
@dhensby

dhensby Aug 13, 2014

Contributor

Makes sense, but this is new work that is actively going to make a jQuery upgrade a pain, you can try using a fallback like this: https://github.com/knockout/knockout/pull/796/files

This comment has been minimized.

Copy link
@tractorcow

tractorcow Aug 13, 2014

Contributor

This whole block can be replaced with parseHTML when we come to update to jquery 1.9.

I've added a ticket to manage this task at #3384.

This comment has been minimized.

Copy link
@dhensby

dhensby Aug 13, 2014

Contributor

Great, sounds sensible

$data = $(jQuery.merge( [], fragment.childNodes ));

// Try and guess the fragment if none is provided
// TODO: data-pjax-fragment might actually give us the fragment. For now we just check most common case
Expand Down

0 comments on commit 98907fb

Please sign in to comment.