Permalink
Browse files

Merge branch 'devel' into release-0.5.3

Pull in fix to #392.
  • Loading branch information...
2 parents 95974f8 + 5eae5ea commit 46e772c0ef7a81bd27e62030c177d8c4e60c5a6a @glasser glasser committed Jan 3, 2013
Showing with 77 additions and 2 deletions.
  1. +25 −2 packages/spark/spark.js
  2. +52 −0 packages/spark/spark_tests.js
View
@@ -543,6 +543,19 @@ var pathForRange = function (r) {
// `range` is a region of `document`. Modify it in-place so that it
// matches the result of Spark.render(htmlFunc), preserving landmarks.
Spark.renderToRange = function (range, htmlFunc) {
+ // `range` may be out-of-document and we don't check here.
+ // XXX should we?
+ //
+ // Explicit finalization of ranges (done within Spark or by a call to
+ // Spark.finalize) prevents us from being called in the first place.
+ // The newly rendered material will be checked to see if it's in the
+ // document by scheduleOnscreenSetUp's scheduled setup.
+ // However, if range is not valid, bail out now before running
+ // htmlFunc.
+ var startNode = range.firstNode();
+ if (! startNode || ! startNode.parentNode)
+ return;
+
var renderer = new Spark._Renderer();
// Call 'func' for each landmark in 'range'. Pass two arguments to
@@ -593,8 +606,18 @@ Spark.renderToRange = function (range, htmlFunc) {
// find preservation roots that come from landmarks enclosing the
// updated region
var walk = range;
- while ((walk = findParentOfType(Spark._ANNOTATION_LANDMARK, walk)))
- pc.addRoot(walk.preserve, range, tempRange, walk.containerNode());
+ while ((walk = walk.findParent())) {
+ if (! walk.firstNode().parentNode)
+ // we're in a DOM island with a top-level range (not really
+ // allowed, but could happen if `range` is on nodes that
+ // manually removed.
+ // XXX check for this sooner; hard to reason about this function
+ // on a "malformed" liverange tree
+ break;
+
+ if (walk.type === Spark._ANNOTATION_LANDMARK, walk)
+ pc.addRoot(walk.preserve, range, tempRange, walk.containerNode());
+ }
pc.addRoot(Spark._globalPreserves, range, tempRange);
@@ -3831,4 +3831,56 @@ Tinytest.add("spark - legacy preserve names", function (test) {
Meteor.flush();
});
+Tinytest.add("spark - update defunct range", function (test) {
+ // Test that Spark doesn't freak out if it tries to update
+ // a LiveRange on nodes that have been taken out of the document.
+ //
+ // See https://github.com/meteor/meteor/issues/392.
+
+ var R = ReactiveVar("foo");
+
+ var div = OnscreenDiv(Spark.render(function () {
+ return "<p>" + Spark.isolate(function() {
+ return R.get();
+ }) + "</p>";
+ }));
+
+ test.equal(div.html(), "<p>foo</p>");
+ R.set("bar");
+ Meteor.flush();
+ test.equal(R.numListeners(), 1);
+ test.equal(div.html(), "<p>bar</p>");
+ test.equal(div.node().firstChild.nodeName, "P");
+ div.node().firstChild.innerHTML = '';
+ R.set("baz");
+ Meteor.flush(); // should throw no errors
+ // will be 1 if our isolate func was run.
+ test.equal(R.numListeners(), 0);
+
+ /////
+
+ R = ReactiveVar("foo");
+
+ div = OnscreenDiv(Spark.render(function () {
+ return "<p>" + Spark.setDataContext(
+ {},
+ "<span>" + Spark.isolate(function() {
+ return R.get();
+ }) + "</span>") + "</p>";
+ }));
+
+ test.equal(div.html(), "<p><span>foo</span></p>");
+ R.set("bar");
+ Meteor.flush();
+ test.equal(R.numListeners(), 1);
+ test.equal(div.html(), "<p><span>bar</span></p>");
+ test.equal(div.node().firstChild.nodeName, "P");
+ div.node().firstChild.innerHTML = '';
+ R.set("baz");
+ Meteor.flush(); // should throw no errors
+ // will be 1 if our isolate func was run.
+ test.equal(R.numListeners(), 0);
+
+});
+
})();

0 comments on commit 46e772c

Please sign in to comment.