Skip to content

Commit

Permalink
Make sure template nodes are created/moved within the correct documen…
Browse files Browse the repository at this point in the history
…t context.
  • Loading branch information
mbest committed Nov 9, 2014
1 parent 4226c01 commit c0ef6ec
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 21 deletions.
2 changes: 2 additions & 0 deletions spec/iframetest.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<!doctype html>
<html><head></head><body></body></html>
32 changes: 27 additions & 5 deletions spec/templatingBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ var dummyTemplateEngine = function (templates) {
return new ko.templateSources.anonymousTemplate(template); // Anonymous template
};

this.renderTemplateSource = function (templateSource, bindingContext, options) {
this.renderTemplateSource = function (templateSource, bindingContext, options, templateDocument) {
var data = bindingContext['$data'];
templateDocument = templateDocument || document;
options = options || {};
var templateText = templateSource.text();
if (typeof templateText == "function")
Expand Down Expand Up @@ -68,14 +69,14 @@ var dummyTemplateEngine = function (templates) {

// Use same HTML parsing code as real template engine so as to trigger same combination of IE weirdnesses
// Also ensure resulting nodelist is an array to mimic what the default templating engine does, so we see the effects of not being able to remove dead memo comment nodes.
return ko.utils.arrayPushAll([], ko.utils.parseHtmlFragment(result));
return ko.utils.arrayPushAll([], ko.utils.parseHtmlFragment(result, templateDocument));
};

this.rewriteTemplate = function (template, rewriterCallback) {
this.rewriteTemplate = function (template, rewriterCallback, templateDocument) {
// Only rewrite if the template isn't a function (can't rewrite those)
var templateSource = this.makeTemplateSource(template);
var templateSource = this.makeTemplateSource(template, templateDocument);
if (typeof templateSource.text() != "function")
return ko.templateEngine.prototype.rewriteTemplate.call(this, template, rewriterCallback);
return ko.templateEngine.prototype.rewriteTemplate.call(this, template, rewriterCallback, templateDocument);
};
this.createJavaScriptEvaluatorBlock = function (script) { return "[js:" + script + "]"; };
};
Expand Down Expand Up @@ -1103,4 +1104,25 @@ describe('Templating', function() {
expect(testDocFrag.childNodes[0].tagName).toEqual("P");
expect(testDocFrag.childNodes[0]).toContainHtml("myval: 123");
});

it('Should be posible to render a template in an iframe', function () {
testNode.innerHTML = '<iframe src="iframetest.html"></iframe>';
var iframe = testNode.childNodes[0], loaded = false;
ko.utils.registerEventHandler(iframe, 'load', function() {
loaded = true;
});

waitsFor(function () {
return loaded;
}, 1000);

runs(function () {
var iframeBody = iframe.contentWindow.document.body;

ko.setTemplateEngine(new dummyTemplateEngine({ someTemplate: "ABC" }));
ko.renderTemplate("someTemplate", null, null, iframeBody);
expect(iframeBody.childNodes.length).toEqual(1);
expect(iframeBody.innerHTML).toEqual("ABC");
});
});
})
5 changes: 3 additions & 2 deletions src/templating/jquery.tmpl/jqueryTmplTemplateEngine.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
return jQueryInstance['tmpl'](compiledTemplate, data, jQueryTemplateOptions);
}

this['renderTemplateSource'] = function(templateSource, bindingContext, options) {
this['renderTemplateSource'] = function(templateSource, bindingContext, options, templateDocument) {
templateDocument = templateDocument || document;
options = options || {};
ensureHasReferencedJQueryTemplates();

Expand All @@ -46,7 +47,7 @@
var jQueryTemplateOptions = jQueryInstance['extend']({ 'koBindingContext': bindingContext }, options['templateOptions']);

var resultNodes = executeTemplate(precompiled, data, jQueryTemplateOptions);
resultNodes['appendTo'](document.createElement("div")); // Using "appendTo" forces jQuery/jQuery.tmpl to perform necessary cleanup work
resultNodes['appendTo'](templateDocument.createElement("div")); // Using "appendTo" forces jQuery/jQuery.tmpl to perform necessary cleanup work

jQueryInstance['fragments'] = {}; // Clear jQuery's fragment cache to avoid a memory leak after a large number of template renders
return resultNodes;
Expand Down
4 changes: 2 additions & 2 deletions src/templating/native/nativeTemplateEngine.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ko.nativeTemplateEngine = function () {

ko.nativeTemplateEngine.prototype = new ko.templateEngine();
ko.nativeTemplateEngine.prototype.constructor = ko.nativeTemplateEngine;
ko.nativeTemplateEngine.prototype['renderTemplateSource'] = function (templateSource, bindingContext, options) {
ko.nativeTemplateEngine.prototype['renderTemplateSource'] = function (templateSource, bindingContext, options, templateDocument) {
var useNodesIfAvailable = !(ko.utils.ieVersion < 9), // IE<9 cloneNode doesn't work properly
templateNodesFunc = useNodesIfAvailable ? templateSource['nodes'] : null,
templateNodes = templateNodesFunc ? templateSource['nodes']() : null;
Expand All @@ -13,7 +13,7 @@ ko.nativeTemplateEngine.prototype['renderTemplateSource'] = function (templateSo
return ko.utils.makeArray(templateNodes.cloneNode(true).childNodes);
} else {
var templateText = templateSource['text']();
return ko.utils.parseHtmlFragment(templateText);
return ko.utils.parseHtmlFragment(templateText, templateDocument);
}
};

Expand Down
5 changes: 3 additions & 2 deletions src/templating/templateEngine.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// // - you might also want to make bindingContext.$parent, bindingContext.$parents,
// // and bindingContext.$root available in the template too
// // - options gives you access to any other properties set on "data-bind: { template: options }"
// // - templateDocument is the document object of the template
// //
// // Return value: an array of DOM nodes
// }
Expand All @@ -26,7 +27,7 @@

ko.templateEngine = function () { };

ko.templateEngine.prototype['renderTemplateSource'] = function (templateSource, bindingContext, options) {
ko.templateEngine.prototype['renderTemplateSource'] = function (templateSource, bindingContext, options, templateDocument) {
throw new Error("Override renderTemplateSource");
};

Expand All @@ -51,7 +52,7 @@ ko.templateEngine.prototype['makeTemplateSource'] = function(template, templateD

ko.templateEngine.prototype['renderTemplate'] = function (template, bindingContext, options, templateDocument) {
var templateSource = this['makeTemplateSource'](template, templateDocument);
return this['renderTemplateSource'](templateSource, bindingContext, options);
return this['renderTemplateSource'](templateSource, bindingContext, options, templateDocument);
};

ko.templateEngine.prototype['isTemplateRewritten'] = function (template, templateDocument) {
Expand Down
20 changes: 11 additions & 9 deletions src/utils.domManipulation.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
(function () {
var leadingCommentRegex = /^(\s*)<!--(.*?)-->/;

function simpleHtmlParse(html) {
function simpleHtmlParse(html, documentContext) {
documentContext || (documentContext = document);

// Based on jQuery's "clean" function, but only accounting for table-related elements.
// If you have referenced jQuery, this won't be used anyway - KO will use jQuery's "clean" function directly

Expand All @@ -11,7 +13,7 @@
// (possibly a text node) in front of the comment. So, KO does not attempt to workaround this IE issue automatically at present.

// Trim whitespace, otherwise indexOf won't work as expected
var tags = ko.utils.stringTrim(html).toLowerCase(), div = document.createElement("div");
var tags = ko.utils.stringTrim(html).toLowerCase(), div = documentContext.createElement("div");

// Finds the first match from the left column, and returns the corresponding "wrap" data from the right column
var wrap = tags.match(/^<(thead|tbody|tfoot)/) && [1, "<table>", "</table>"] ||
Expand All @@ -35,13 +37,13 @@
return ko.utils.makeArray(div.lastChild.childNodes);
}

function jQueryHtmlParse(html) {
function jQueryHtmlParse(html, documentContext) {
// jQuery's "parseHTML" function was introduced in jQuery 1.8.0 and is a documented public API.
if (jQueryInstance['parseHTML']) {
return jQueryInstance['parseHTML'](html) || []; // Ensure we always return an array and never null
return jQueryInstance['parseHTML'](html, documentContext) || []; // Ensure we always return an array and never null
} else {
// For jQuery < 1.8.0, we fall back on the undocumented internal "clean" function.
var elems = jQueryInstance['clean']([html]);
var elems = jQueryInstance['clean']([html], documentContext);

// As of jQuery 1.7.1, jQuery parses the HTML by appending it to some dummy parent nodes held in an in-memory document fragment.
// Unfortunately, it never clears the dummy parent nodes from the document fragment, so it leaks memory over time.
Expand All @@ -60,9 +62,9 @@
}
}

ko.utils.parseHtmlFragment = function(html) {
return jQueryInstance ? jQueryHtmlParse(html) // As below, benefit from jQuery's optimisations where possible
: simpleHtmlParse(html); // ... otherwise, this simple logic will do in most common cases.
ko.utils.parseHtmlFragment = function(html, documentContext) {
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.
};

ko.utils.setHtml = function(node, html) {
Expand All @@ -82,7 +84,7 @@
jQueryInstance(node)['html'](html);
} else {
// ... otherwise, use KO's own parsing logic.
var parsedNodes = ko.utils.parseHtmlFragment(html);
var parsedNodes = ko.utils.parseHtmlFragment(html, node.ownerDocument);
for (var i = 0; i < parsedNodes.length; i++)
node.appendChild(parsedNodes[i]);
}
Expand Down
3 changes: 2 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,9 @@ ko.utils = (function () {
// Ensure it's a real array, as we're about to reparent the nodes and
// we don't want the underlying collection to change while we're doing that.
var nodesArray = ko.utils.makeArray(nodes);
var templateDocument = (nodesArray[0] && nodesArray[0].ownerDocument) || document;

var container = document.createElement('div');
var container = templateDocument.createElement('div');
for (var i = 0, j = nodesArray.length; i < j; i++) {
container.appendChild(ko.cleanNode(nodesArray[i]));
}
Expand Down

0 comments on commit c0ef6ec

Please sign in to comment.