diff --git a/packages/ui/dombackend.js b/packages/ui/dombackend.js index 7800723175..d94d8fbcc0 100644 --- a/packages/ui/dombackend.js +++ b/packages/ui/dombackend.js @@ -28,7 +28,7 @@ if (Meteor.isClient) { // Causes `elem` (a DOM element) to be detached from its parent, if any. // Whether or not `elem` was detached, causes any callbacks registered - // with `onRemoveElement` on `elem` and its descendants to fire. + // with `onElementTeardown` on `elem` and its descendants to fire. // Not for use on non-element nodes. // // This method is modeled after the behavior of jQuery's `$(elem).remove()`, @@ -37,11 +37,17 @@ if (Meteor.isClient) { $jq(elem).remove(); }; + DomBackend.tearDownElement = function (elem) { + var elems = Array.prototype.slice.call(elem.getElementsByTagName('*')); + elems.push(elem); + $jq.cleanData(elems); + }; + // Registers a callback function to be called when the given element or // one of its ancestors is removed from the DOM via the backend library. // The callback function is called at most once, and it receives the element // in question as an argument. - DomBackend.onRemoveElement = function (elem, func) { + DomBackend.onElementTeardown = function (elem, func) { if (! elem[REMOVAL_CALLBACKS_PROPERTY_NAME]) { elem[REMOVAL_CALLBACKS_PROPERTY_NAME] = []; diff --git a/packages/ui/dombackend_tests.js b/packages/ui/dombackend_tests.js index 1b3583b192..5799e3c030 100644 --- a/packages/ui/dombackend_tests.js +++ b/packages/ui/dombackend_tests.js @@ -35,16 +35,16 @@ var isDetachedSingleNode = function (test, node) { }; Tinytest.add("ui - DomBackend - element removal", function (test) { - // Test that calling removeElement on a detached element calls onRemoveElement + // Test that calling removeElement on a detached element calls onElementTeardown // on it and its descendents. For jQuery, `removeElement` runs `$(elem).remove()`, // so it tests detecting a jQuery removal, as well as the stronger condition // that clean-up still happens on the DOM tree in the detached case. runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) { - DomBackend.onRemoveElement(div, func1); - DomBackend.onRemoveElement(span, func2); - DomBackend.onRemoveElement(b, func3); + DomBackend.onElementTeardown(div, func1); + DomBackend.onElementTeardown(span, func2); + DomBackend.onElementTeardown(b, func3); // test second callback on same element - DomBackend.onRemoveElement(div, func4); + DomBackend.onElementTeardown(div, func4); DomBackend.removeElement(div); // "remove" the (parentless) DIV @@ -59,10 +59,10 @@ Tinytest.add("ui - DomBackend - element removal", function (test) { // Test that `removeElement` actually removes the element // (and fires appropriate callbacks). runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) { - DomBackend.onRemoveElement(div, func1); - DomBackend.onRemoveElement(span, func2); - DomBackend.onRemoveElement(b, func3); - DomBackend.onRemoveElement(div, func4); + DomBackend.onElementTeardown(div, func1); + DomBackend.onElementTeardown(span, func2); + DomBackend.onElementTeardown(b, func3); + DomBackend.onElementTeardown(div, func4); DomBackend.removeElement(span); // remove the SPAN @@ -83,10 +83,10 @@ Tinytest.add("ui - DomBackend - element removal (jQuery)", function (test) { // Test with `$(elem).remove()`. runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) { - DomBackend.onRemoveElement(div, func1); - DomBackend.onRemoveElement(span, func2); - DomBackend.onRemoveElement(b, func3); - DomBackend.onRemoveElement(div, func4); + DomBackend.onElementTeardown(div, func1); + DomBackend.onElementTeardown(span, func2); + DomBackend.onElementTeardown(b, func3); + DomBackend.onElementTeardown(div, func4); $(span).remove(); // remove the SPAN @@ -103,10 +103,10 @@ Tinytest.add("ui - DomBackend - element removal (jQuery)", function (test) { // Test that `$(elem).detach()` is NOT considered a removal. runDivSpanBTest(function (div, span, b, buf, func1, func2, func3, func4) { - DomBackend.onRemoveElement(div, func1); - DomBackend.onRemoveElement(span, func2); - DomBackend.onRemoveElement(b, func3); - DomBackend.onRemoveElement(div, func4); + DomBackend.onElementTeardown(div, func1); + DomBackend.onElementTeardown(span, func2); + DomBackend.onElementTeardown(b, func3); + DomBackend.onElementTeardown(div, func4); $(span).detach(); // detach the SPAN diff --git a/packages/ui/domrange.js b/packages/ui/domrange.js index 1b46b5cae1..a875b759de 100644 --- a/packages/ui/domrange.js +++ b/packages/ui/domrange.js @@ -11,7 +11,7 @@ var removeNode = function (n) { n.parentNode._uihooks && n.parentNode._uihooks.removeElement) { n.parentNode._uihooks.removeElement(n); } else { - DomBackend.removeElement(n); + n.parentNode.removeChild(n); } }; @@ -110,8 +110,8 @@ var rangeParented = function (range) { range._rangeDict = rangeDict; // get jQuery to tell us when this node is removed - DomBackend.onRemoveElement(parentNode, function () { - rangeRemoved(range, true /* elementsAlreadyRemoved */); + DomBackend.onElementTeardown(parentNode, function () { + rangeRemoved(range, true /* alreadyTornDown */); }); } @@ -128,7 +128,7 @@ var rangeParented = function (range) { } }; -var rangeRemoved = function (range, elementsAlreadyRemoved) { +var rangeRemoved = function (range, alreadyTornDown) { if (! range.isRemoved) { range.isRemoved = true; @@ -146,29 +146,39 @@ var rangeRemoved = function (range, elementsAlreadyRemoved) { if (range.removed) range.removed(); - membersRemoved(range, elementsAlreadyRemoved); + membersRemoved(range, alreadyTornDown); } }; -var nodeRemoved = function (node, elementsAlreadyRemoved) { +var nodeRemoved = function (node, alreadyTornDown) { if (node.nodeType === 1) { // ELEMENT var comps = DomRange.getComponents(node); for (var i = 0, N = comps.length; i < N; i++) - rangeRemoved(comps[i], elementsAlreadyRemoved); + rangeRemoved(comps[i], true /* alreadyTornDown */); - if (! elementsAlreadyRemoved) - DomBackend.removeElement(node); + // `alreadyTornDown` is an optimization so that we don't + // tear down the same elements multiple times when tearing + // down a tree of DomRanges and elements, leading to asymptotic + // inefficiency. + // + // When jQuery removes an element or DomBackend.tearDownElement + // is called, the DOM is "cleaned" recursively, calling all + // onElementTearDown handlers on the entire DOM subtree. + // Since the entire subtree is already walked, we don't want to + // also walk the subtrees of each DomRange for teardown purposes. + if (! alreadyTornDown) + DomBackend.tearDownElement(node); } }; -var membersRemoved = function (range, elementsAlreadyRemoved) { +var membersRemoved = function (range, alreadyTornDown) { var members = range.members; for (var k in members) { var mem = members[k]; if (mem instanceof DomRange) - rangeRemoved(mem, elementsAlreadyRemoved); + rangeRemoved(mem, alreadyTornDown); else - nodeRemoved(mem, elementsAlreadyRemoved); + nodeRemoved(mem, alreadyTornDown); } }; @@ -230,7 +240,7 @@ _extend(DomRange.prototype, { for (var i = 0, N = nodes.length; i < N; i++) removeNode(nodes[i]); - membersRemoved(this, true /* elementsAlreadyRemoved */); + membersRemoved(this); this.members = {}; }, @@ -341,7 +351,7 @@ _extend(DomRange.prototype, { removeNode(this.start); removeNode(this.end); this.owner = null; - rangeRemoved(this, true /* elementsAlreadyRemoved */); + rangeRemoved(this); return; } diff --git a/packages/ui/domrange_tests.js b/packages/ui/domrange_tests.js index fb042f2dea..2fd7f2b519 100644 --- a/packages/ui/domrange_tests.js +++ b/packages/ui/domrange_tests.js @@ -25,7 +25,8 @@ var inDocument = function (range, func) { try { func(range); } finally { - document.body.removeChild(onscreen); + if (onscreen.parentNode === document.body) + document.body.removeChild(onscreen); } }; @@ -862,7 +863,7 @@ Tinytest.add("ui - DomRange - structural removal", function (test) { test.isTrue(e.isRemoved); - for (var scenario = 0; scenario < 2; scenario++) { + for (var scenario = 0; scenario < 3; scenario++) { var f = new DomRange; var g = document.createElement("DIV"); var h = new DomRange; @@ -882,6 +883,8 @@ Tinytest.add("ui - DomRange - structural removal", function (test) { r.removeAll(); else if (scenario === 1) r.remove('f'); + else if (scenario === 2) + $(r.parentNode()).remove(); test.isTrue(f.isRemoved); test.isTrue(h.isRemoved); test.isTrue(k.isRemoved); diff --git a/packages/ui/render.js b/packages/ui/render.js index 31da03bda3..58d30787dc 100644 --- a/packages/ui/render.js +++ b/packages/ui/render.js @@ -382,7 +382,7 @@ var materialize = function (node, parent, before, parentComponent) { reportUIException(e); } }); - UI.DomBackend.onRemoveElement(elem, function () { + UI.DomBackend.onElementTeardown(elem, function () { attrComp.stop(); }); }