From 0efe0e26d37fa08d419926b4ab3ecd8de5e34853 Mon Sep 17 00:00:00 2001
From: David Greenspan <dgreenspan@alum.mit.edu>
Date: Wed, 22 Aug 2012 15:15:41 -0700
Subject: [PATCH] Spark.labelBranch safe on non-element-balanced HTML

---
 packages/spark/spark.js       | 19 +++++++++++++++++
 packages/spark/spark_tests.js | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/packages/spark/spark.js b/packages/spark/spark.js
index 0206f9c914..a41bfbc1be 100644
--- a/packages/spark/spark.js
+++ b/packages/spark/spark.js
@@ -166,6 +166,17 @@ _.extend(Spark._Renderer.prototype, {
       getNotes: function () {
         var top = stack[stack.length - 1];
         return top;
+      },
+      // Mark this branch with `getNotes()[prop] = true` and also
+      // walk up the stack marking parent branches (until an
+      // existing truthy value for `prop` is found).
+      // This makes it easy to test whether any descendent of a
+      // branch has the mark.
+      mark: function (prop) {
+        for (var i = stack.length - 1;
+             i >= 0 && ! stack[i][prop];
+             i--)
+          stack[i][prop] = true;
       }
     };
   },
@@ -990,8 +1001,15 @@ Spark.labelBranch = function (label, htmlFunc) {
 
   renderer.currentBranch.pushLabel(label);
   var html = htmlFunc();
+  var occupied = renderer.currentBranch.getNotes().occupied;
   renderer.currentBranch.popLabel();
 
+  if (! occupied)
+    // don't create annotation if branch doesn't contain any landmarks.
+    // if this label isn't on an element-level HTML boundary, then that
+    // is certainly the case.
+    return html;
+
   return renderer.annotate(
     html, Spark._ANNOTATION_LABEL, { label: label });
 
@@ -1032,6 +1050,7 @@ Spark.createLandmark = function (options, htmlFunc) {
     if (typeof preserve[selector] !== 'function')
       preserve[selector] = function () { return true; };
 
+  renderer.currentBranch.mark('occupied');
   var notes = renderer.currentBranch.getNotes();
   var landmark;
   if (notes.originalRange) {
diff --git a/packages/spark/spark_tests.js b/packages/spark/spark_tests.js
index 068c85fe26..497bd04ce4 100644
--- a/packages/spark/spark_tests.js
+++ b/packages/spark/spark_tests.js
@@ -3452,3 +3452,42 @@ Tinytest.add("spark - landmark preserve", function (test) {
   div.kill();
   Meteor.flush();
 });
+
+Tinytest.add("spark - branch annotation is optional", function (test) {
+  // test that labelBranch works on HTML that isn't element-balanced
+  // and doesn't fail by trying to emit an annotation when it contains
+  // no landmarks.
+
+  var R = ReactiveVar("foo");
+
+  var Rget = function () { return R.get(); };
+  var cnst = function (c) { return function () { return c; }; };
+  var lmhr = function () {
+    return Spark.createLandmark({preserve:['hr']}, function () {
+      return '<hr/>';
+    });
+  };
+
+  var div = OnscreenDiv(Meteor.render(function () {
+    return '<div class="' + Spark.labelBranch('A', Rget) + '">' +
+      Spark.labelBranch('B', cnst('</div><div>')) +
+      Spark.labelBranch('C', lmhr) + Spark.labelBranch('D', lmhr) +
+      '</div>';
+  }));
+
+  test.equal(div.html(), '<div class="foo"></div><div><hr><hr></div>');
+  var div1 = div.node().firstChild;
+  var hrs1 = DomUtils.findAll(div.node(), 'hr');
+  R.set("bar");
+  Meteor.flush();
+  test.equal(div.html(), '<div class="bar"></div><div><hr><hr></div>');
+  var div2 = div.node().firstChild;
+  var hrs2 = DomUtils.findAll(div.node(), 'hr');
+
+  test.isFalse(div1 === div2);
+  test.isTrue(hrs1[0] === hrs2[0]);
+  test.isTrue(hrs1[1] === hrs2[1]);
+
+  div.kill();
+  Meteor.flush();
+});