Skip to content

Commit

Permalink
Merge pull request knockout#1880 from knockout/1880-revert-to-jquery-…
Browse files Browse the repository at this point in the history
…parsing

Removing jQuery HTML parsing by default is breaking
  • Loading branch information
mbest committed Sep 28, 2015
2 parents 8decc43 + b85e327 commit a2d8d56
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 21 deletions.
2 changes: 1 addition & 1 deletion spec/lib/loadDependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
if (url) {
var shouldInclude = getParam(name);
if ((dependency.include || shouldInclude) && shouldInclude !== "0" && shouldInclude !== "false") {
if (shouldInclude && /^[0-9]+\.[0-9.]+$/.test(shouldInclude)) {
if (shouldInclude && shouldInclude !== "1" && shouldInclude !== "true") {
url = url.replace(dependency.versionString || 'latest', shouldInclude);
}
jasmine.addScriptReference(url);
Expand Down
20 changes: 9 additions & 11 deletions spec/parseHtmlFragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ describe('Parse HTML fragment', function() {

ko.utils.arrayForEach(
[
{ html: '<tr-component></tr-component>', parsed: ['<tr-component></tr-component>'] },
{ html: '<tr-component></tr-component>', parsed: ['<tr-component></tr-component>'], jQueryRequiredVersion: "3.0" },
{ html: '<thead><tr><th><thcomponent>hello</thcomponent></th></tr></thead>', parsed: ['<thead><tr><th><thcomponent>hello</thcomponent></th></tr></thead>'], ignoreRedundantTBody: true },
{ html: '<tbody-component>world</tbody-component>', parsed: ['<tbody-component>world</tbody-component>'], minSupportedIEVersion: 8 },
{ html: '<tfoot-component>foo</tfoot-component>', parsed: ['<tfoot-component>foo</tfoot-component>'] },
{ html: '<tbody-component>world</tbody-component>', parsed: ['<tbody-component>world</tbody-component>'], jQueryRequiredVersion: "3.0" },
{ html: '<tfoot-component>foo</tfoot-component>', parsed: ['<tfoot-component>foo</tfoot-component>'], jQueryRequiredVersion: "3.0" },
{ html: '<div></div>', parsed: ['<div></div>'] },
{ html: '<custom-component></custom-component>', parsed: ['<custom-component></custom-component>'] },
{ html: '<tr></tr>', parsed: ['<tr></tr>'] },
Expand All @@ -27,6 +27,12 @@ describe('Parse HTML fragment', function() {
{ html: '<optgroup label=x><option>text</option></optgroup>', parsed: ['<optgroup label=x><option>text</option></optgroup>'] },
{ html: '<option>text</option>', parsed: [ '<option>text</option>' ] }
], function (data) {
// jQuery's HTML parsing fails on element names like tr-* (but this is fixed in jQuery 3.x).
if (window.jQuery && data.jQueryRequiredVersion && jQuery.fn.jquery < data.jQueryRequiredVersion) {
it('unsupported environment for parsing ' + data.html, function () { });
return;
}

it('should parse ' + data.html + ' correctly', function () {
// IE 6-8 has a lot of trouble with custom elements. We have several strategies for dealing with
// this, each involving different (awkward) requirements for the application.
Expand Down Expand Up @@ -55,14 +61,6 @@ describe('Parse HTML fragment', function() {
if (window.innerShiv) {
window.innerShiv.reset();
}

// Out of all the combinations above, there is still one edge case we can't support without
// dropping jQuery HTML parsing altogether. That is, if you're using jQuery, its parser fails
// on elements named 'tbody-*', on IE 6 and 7 (but it works on IE 8+). This is such an extreme
// edge case that it's preferable to leave this element name unsupported.
if (jasmine.ieVersion < data.minSupportedIEVersion) {
return;
}
}

var parsedNodes = ko.utils.parseHtmlFragment(data.html, document);
Expand Down
12 changes: 3 additions & 9 deletions src/utils.domManipulation.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,7 @@
},

// This is needed for old IE if you're *not* using either jQuery or innerShiv. Doesn't affect other cases.
mayRequireCreateElementHack = ko.utils.ieVersion <= 8,

// We prefer not to use jQuery's HTML parsing, because it fails on element names like tr-*, even
// on the latest browsers (not even just on IE). But we retain use of jQuery HTML parsing for old
// IE, to avoid breaking compatibility with parsing edge-cases. Strangely, jQuery's HTML parsing
// works OK on elements named tr-* on old IE browsers.
allowJQueryHtmlParsing = ko.utils.ieVersion <= 8;
mayRequireCreateElementHack = ko.utils.ieVersion <= 8;

function getWrap(tags) {
var m = tags.match(/^<([a-z]+)[ >]/);
Expand Down Expand Up @@ -101,7 +95,7 @@
}

ko.utils.parseHtmlFragment = function(html, documentContext) {
return allowJQueryHtmlParsing && jQueryInstance ?
return jQueryInstance ?
jQueryHtmlParse(html, documentContext) : // As below, benefit from jQuery's optimisations where possible
simpleHtmlParse(html, documentContext); // ... otherwise, this simple logic will do in most common cases.
};
Expand All @@ -119,7 +113,7 @@
// jQuery contains a lot of sophisticated code to parse arbitrary HTML fragments,
// for example <tr> elements which are not normally allowed to exist on their own.
// If you've referenced jQuery we'll use that rather than duplicating its code.
if (allowJQueryHtmlParsing && jQueryInstance) {
if (jQueryInstance) {
jQueryInstance(node)['html'](html);
} else {
// ... otherwise, use KO's own parsing logic.
Expand Down

0 comments on commit a2d8d56

Please sign in to comment.