rewrite after/prepend/before/append() to fix eval

In some of these operations <script> tags would be eval'ed in the wrong order. The rewrite ensures a consistent eval order and improves code readability. Closes madrobby#550
jasondscott · Sep 30, 2012 · 17f69f4 · 17f69f4
1 parent 85c8a84
commit 17f69f4
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 38 deletions.
diff --git a/src/zepto.js b/src/zepto.js
@@ -591,46 +591,50 @@ var Zepto = (function() {
     }
   })
 
-  function insert(operator, target, node) {
-    var parent = (operator % 2) ? target : target.parentNode
-    parent ? parent.insertBefore(node,
-      !operator ? target.nextSibling :      // after
-      operator == 1 ? parent.firstChild :   // prepend
-      operator == 2 ? target :              // before
-      null) :                               // append
-      $(node).remove()
-  }
-
   function traverseNode(node, fun) {
     fun(node)
     for (var key in node.childNodes) traverseNode(node.childNodes[key], fun)
   }
 
   // Generate the `after`, `prepend`, `before`, `append`,
   // `insertAfter`, `insertBefore`, `appendTo`, and `prependTo` methods.
-  adjacencyOperators.forEach(function(key, operator) {
-    $.fn[key] = function(){
+  adjacencyOperators.forEach(function(operator, operatorIndex) {
+    var inside = operatorIndex % 2 //=> prepend, append
+
+    $.fn[operator] = function(){
       // arguments can be nodes, arrays of nodes, Zepto objects and HTML strings
-      var nodes = $.map(arguments, function(n){ return isObject(n) ? n : zepto.fragment(n) })
+      var nodes = $.map(arguments, function(n){ return isObject(n) ? n : zepto.fragment(n) }),
+          parent, copyByClone = this.length > 1
       if (nodes.length < 1) return this
-      var size = this.length, copyByClone = size > 1, inReverse = operator < 2
-
-      return this.each(function(index, target){
-        for (var i = 0; i < nodes.length; i++) {
-          var node = nodes[inReverse ? nodes.length-i-1 : i]
-          traverseNode(node, function(node){
-            if (node.nodeName != null && node.nodeName.toUpperCase() === 'SCRIPT' &&
-               (!node.type || node.type === 'text/javascript') && !node.src)
-              window['eval'].call(window, node.innerHTML)
+
+      return this.each(function(_, target){
+        parent = inside ? target : target.parentNode
+
+        // convert all methods to a "before" operation
+        target = operatorIndex == 0 ? target.nextSibling :
+                 operatorIndex == 1 ? target.firstChild :
+                 operatorIndex == 2 ? target :
+                 null
+
+        nodes.forEach(function(node){
+          if (copyByClone) node = node.cloneNode(true)
+          else if (!parent) return $(node).remove()
+
+          traverseNode(parent.insertBefore(node, target), function(el){
+            if (el.nodeName != null && el.nodeName.toUpperCase() === 'SCRIPT' &&
+               (!el.type || el.type === 'text/javascript') && !el.src)
+              window['eval'].call(window, el.innerHTML)
           })
-          if (copyByClone && index < size - 1) node = node.cloneNode(true)
-          insert(operator, target, node)
-        }
+        })
       })
     }
 
-    $.fn[(operator % 2) ? key+'To' : 'insert'+(operator ? 'Before' : 'After')] = function(html){
-      $(html)[key](this)
+    // after    => insertAfter
+    // prepend  => prependTo
+    // before   => insertBefore
+    // append   => appendTo
+    $.fn[inside ? operator+'To' : 'insert'+(operatorIndex ? 'Before' : 'After')] = function(html){
+      $(html)[operator](this)
       return this
     }
   })

diff --git a/test/zepto.html b/test/zepto.html
@@ -1781,14 +1781,13 @@ <h1>Zepto DOM unit tests</h1>
       },
 
       testAppendEval: function (t) {
+        window.someGlobalVariable = 0
         try {
-          window.someGlobalVariable = false
-          $('<' + 'script>window.someGlobalVariable = true</script' + '>').appendTo('body')
-          t.assert(window.someGlobalVariable)
-
-          window.someGlobalVariable = false
-          $('<' + 'script>this.someGlobalVariable = true</script' + '>').appendTo('body')
-          t.assert(window.someGlobalVariable)
+          $('#fixtures').append(
+            '<div><b id="newByAppend">Hi</b>' +
+            '<\script>this.someGlobalVariable += $("#newByAppend").size()<\/script></div>'
+          )
+          t.assertIdentical(1, window.someGlobalVariable)
         } finally {
           delete window.someGlobalVariable
         }
@@ -1805,11 +1804,26 @@ <h1>Zepto DOM unit tests</h1>
       },
 
       testHtmlEval: function (t) {
+        window.someGlobalVariable = 0
         try {
-          window.someGlobalVariable = false
-          $('<div></div>').appendTo('body')
-            .html('<' + 'script>window.someGlobalVariable = true</script' + '>')
-          t.assert(window.someGlobalVariable)
+          $('<div>').appendTo('#fixtures').html(
+            '<div><b id="newByHtml">Hi</b>' +
+            '<\script>this.someGlobalVariable += $("#newByHtml").size()<\/script></div>'
+          )
+          t.assertIdentical(1, window.someGlobalVariable)
+        } finally {
+          delete window.someGlobalVariable
+        }
+      },
+
+      testPrependEval: function (t) {
+        window.someGlobalVariable = 0
+        try {
+          $('<div>').appendTo('#fixtures').prepend(
+            '<b id="newByPrepend">Hi</b>' +
+            '<\script>this.someGlobalVariable += $("#newByPrepend").size()<\/script>'
+          )
+          t.assertIdentical(1, window.someGlobalVariable)
         } finally {
           delete window.someGlobalVariable
         }