Changeset 4062

Timestamp:

12/07/07 01:52:21 (8 months ago)

Author:

davidserduke

Message:

Fixed #1039 and #1733 by going through the core API and making them text node and comment node safe.

Location:

trunk/jquery

Files:

• src/core.js (modified) (7 diffs)
• src/event.js (modified) (5 diffs)
• src/fx.js (modified) (2 diffs)
• src/selector.js (modified) (1 diff)
• test/index.html (modified) (1 diff)
• test/unit/core.js (modified) (33 diffs)
• test/unit/event.js (modified) (2 diffs)
• test/unit/fx.js (modified) (2 diffs)

trunk/jquery/src/core.js

@@ -252 +252 @@
     append: function() {
         return this.domManip(arguments, true, false, function(elem){
-            this.appendChild( elem );
+            if (this.nodeType == 1)
+                this.appendChild( elem );
         });
     },
@@ -258 +259 @@
     prepend: function() {
         return this.domManip(arguments, true, true, function(elem){
-            this.insertBefore( elem, this.firstChild );
+            if (this.nodeType == 1)
+                this.insertBefore( elem, this.firstChild );
         });
     },
@@ -403 +405 @@
         } else
             return this.each(function(){
+                if ( this.nodeType != 1 )
+                    return;
+
                 if ( value.constructor == Array && /radio|checkbox/.test( this.type ) )
                     this.checked = (jQuery.inArray(this.value, value) >= 0 ||
@@ -723 +728 @@
         add: function( elem, classNames ) {
             jQuery.each((classNames || "").split(/\s+/), function(i, className){
-                if ( !jQuery.className.has( elem.className, className ) )
+                if ( elem.nodeType == 1 && !jQuery.className.has( elem.className, className ) )
                     elem.className += (elem.className ? " " : "") + className;
             });
@@ -730 +735 @@
         // internal only, use removeClass("class")
         remove: function( elem, classNames ) {
-            elem.className = classNames != undefined ?
-                jQuery.grep(elem.className.split(/\s+/), function(className){
-                    return !jQuery.className.has( classNames, className );  
-                }).join(" ") :
-                "";
+            if (elem.nodeType == 1)
+                elem.className = classNames != undefined ?
+                    jQuery.grep(elem.className.split(/\s+/), function(className){
+                        return !jQuery.className.has( classNames, className );  
+                    }).join(" ") :
+                    "";
         },
 
@@ -1015 +1021 @@
     
     attr: function( elem, name, value ) {
+        // don't set attributes on text and comment nodes
+        if (!elem || elem.nodeType == 3 || elem.nodeType == 8)
+            return undefined;
+
         var fix = jQuery.isXMLDoc( elem ) ?
             {} :
@@ -1268 +1278 @@
     removeAttr: function( name ) {
         jQuery.attr( this, name, "" );
-        this.removeAttribute( name );
+        if (this.nodeType == 1) 
+            this.removeAttribute( name );
     },
 

trunk/jquery/src/event.js

@@ -9 +9 @@
     // Original by Dean Edwards
     add: function(element, type, handler, data) {
+        if ( element.nodeType == 3 || element.nodeType == 8 )
+            return;
+
         // For whatever reason, IE has trouble passing the window object
         // around, causing it to be cloned in the process
@@ -84 +87 @@
     // Detach an event or set of events from an element
     remove: function(element, type, handler) {
+        // don't do events on text and comment nodes
+        if ( element.nodeType == 3 || element.nodeType == 8 )
+            return;
+
         var events = jQuery.data(element, "events"), ret, index;
 
@@ -148 +155 @@
         // Handle triggering a single element
         } else {
+            // don't do events on text and comment nodes
+            if ( element.nodeType == 3 || element.nodeType == 8 )
+                return;
+
             var val, ret, fn = jQuery.isFunction( element[ type ] || null ),
                 // Check to see if we need to provide a fake event, or not
@@ -274 +285 @@
             var doc = document.documentElement, body = document.body;
             event.pageX = event.clientX + (doc && doc.scrollLeft || body && body.scrollLeft || 0) - (doc.clientLeft || 0);
-            event.pageY = event.clientY + (doc && doc.scrollTop  || body && body.scrollTop  || 0) - (doc.clientLeft || 0);
+            event.pageY = event.clientY + (doc && doc.scrollTop || body && body.scrollTop || 0) - (doc.clientLeft || 0);
         }
             
@@ -438 +449 @@
     // If Safari or IE is used
     // Continually check to see if the document is ready
-    if  (jQuery.browser.msie || jQuery.browser.safari ) (function(){
+    if (jQuery.browser.msie || jQuery.browser.safari ) (function(){
         try {
             // If IE is used, use the trick by Diego Perini

trunk/jquery/src/fx.js

@@ -70 +70 @@
 
         return this[ optall.queue === false ? "each" : "queue" ](function(){
+            if ( this.nodeType != 1)
+                return false;
+
             var opt = jQuery.extend({}, optall);
             var hidden = jQuery(this).is(":hidden"), self = this;
@@ -136 +139 @@
 
         return this.each(function(){
+            if ( this.nodeType != 1)
+                return;
+
             if ( fn.constructor == Array )
                 queue(this, type, fn);

trunk/jquery/src/selector.js

@@ -97 +97 @@
             return [ t ];
 
-        // Make sure that the context is a DOM Element
-        if ( context && !context.nodeType )
-            context = null;
+        // check to make sure context is a DOM element or a document
+        if ( context && context.nodeType != 1 && context.nodeType != 9)
+            return [ ];
 
         // Set the correct context (if none is provided)

trunk/jquery/test/index.html

@@ -172 +172 @@
                 </div>
             </form>
+            <div id="nonnodes"><span>hi</span> there <!-- mon ami --></div>
         </div>
     </div>

trunk/jquery/test/unit/core.js

@@ -142 +142 @@
         ok( jQuery.isFunction(fn), "Recursive Function Call" );
 
-        fn({ some: "data" });
+        fn({ some: "data" });
     };
 
     callme(function(){
-        callme(function(){});
+        callme(function(){});
     });
 });
@@ -153 +153 @@
 
 test("$('html')", function() {
-    expect(4);
+    expect(5);
 
     reset();
@@ -167 +167 @@
     
     reset();
+
+    var j = $("<span>hi</span> there <!-- mon ami -->");
+    ok( j.length >= 2, "Check node,textnode,comment creation (some browsers delete comments)" );
 });
 
@@ -233 +236 @@
     var pass = true;
     for ( var i = 0; i < div.size(); i++ ) {
-      if ( div.get(i).foo != "zoo" ) pass = false;
+        if ( div.get(i).foo != "zoo" ) pass = false;
     }
     ok( pass, "Execute a function, Relative" );
@@ -278 +281 @@
 
 if ( !isLocal ) {
-    test("attr(String) in XML Files", function() {
-        expect(2);
-        stop();
-        $.get("data/dashboard.xml", function(xml) {
-            ok( $("locations", xml).attr("class") == "foo", "Check class attribute in XML document" );
-            ok( $("location", xml).attr("for") == "bar", "Check for attribute in XML document" );
-            start();
-        });
-    });
+    test("attr(String) in XML Files", function() {
+        expect(2);
+        stop();
+        $.get("data/dashboard.xml", function(xml) {
+            ok( $("locations", xml).attr("class") == "foo", "Check class attribute in XML document" );
+            ok( $("location", xml).attr("for") == "bar", "Check for attribute in XML document" );
+            start();
+        });
+    });
 }
 
@@ -299 +302 @@
     var pass = true;
     $("div").attr({foo: 'baz', zoo: 'ping'}).each(function(){
-      if ( this.getAttribute('foo') != "baz" && this.getAttribute('zoo') != "ping" ) pass = false;
+        if ( this.getAttribute('foo') != "baz" && this.getAttribute('zoo') != "ping" ) pass = false;
     });
     ok( pass, "Set Multiple Attributes" );
@@ -305 +308 @@
 
 test("attr(String, Object)", function() {
-    expect(16);
+    expect(17);
     var div = $("div");
     div.attr("foo", "bar");
     var pass = true;
     for ( var i = 0; i < div.size(); i++ ) {
-      if ( div.get(i).getAttribute('foo') != "bar" ) pass = false;
+        if ( div.get(i).getAttribute('foo') != "bar" ) pass = false;
     }
     ok( pass, "Set Attribute" );
@@ -339 +342 @@
     equals( $("#name").attr('someAttr'), 1, 'Set attribute to the number 1' );
 
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+
+    j.attr("name", "attrvalue");
+    equals( j.attr("name"), "attrvalue", "Check node,textnode,comment for attr" );
+    j.removeAttr("name")
+
     reset();
 
@@ -363 +373 @@
 
 if ( !isLocal ) {
-    test("attr(String, Object) - Loaded via XML document", function() {
-        expect(2);
-        stop();
-        $.get('data/dashboard.xml', function(xml) { 
-              var titles = [];
-              $('tab', xml).each(function() {
-                    titles.push($(this).attr('title'));
-              });
-              equals( titles[0], 'Location', 'attr() in XML context: Check first title' );
-              equals( titles[1], 'Users', 'attr() in XML context: Check second title' );
-              start();
-        });
-    });
+    test("attr(String, Object) - Loaded via XML document", function() {
+        expect(2);
+        stop();
+        $.get('data/dashboard.xml', function(xml) { 
+            var titles = [];
+            $('tab', xml).each(function() {
+                titles.push($(this).attr('title'));
+            });
+            equals( titles[0], 'Location', 'attr() in XML context: Check first title' );
+            equals( titles[1], 'Users', 'attr() in XML context: Check second title' );
+            start();
+        });
+    });
 }
 
@@ -409 +419 @@
 
 test("css(String, Object)", function() {
-    expect(19);
+    expect(20);
     ok( $('#foo').is(':visible'), 'Modifying CSS display: Assert element is visible');
     $('#foo').css('display', 'none');
@@ -438 +448 @@
     }
     equals( $('#foo').css('opacity'), '1', "Assert opacity is 1 when a different filter is set in IE, #1438" );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.css("padding-left", "1px");
+    equals( j.css("padding-left"), "1px", "Check node,textnode,comment css works" );
 });
 
@@ -468 +483 @@
 
 test("wrap(String|Element)", function() {
-    expect(6);
+    expect(8);
     var defaultText = 'Try them out:'
     var result = $('#first').wrap('<div class="red"><span></span></div>').text();
@@ -487 +502 @@
         ok( checkbox.checked, "Checkbox's state is erased after wrap() action, see #769" );
     }).click();
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.wrap("<i></i>");
+    equals( $("#nonnodes > i").length, 3, "Check node,textnode,comment wraps ok" );
+    equals( $("#nonnodes > i").text(), j.text() + j[1].nodeValue, "Check node,textnode,comment wraps doesn't hurt text" );
 });
 
@@ -526 +547 @@
 
 test("append(String|Element|Array&lt;Element&gt;|jQuery)", function() {
-    expect(18);
+    expect(21);
     var defaultText = 'Try them out:'
     var result = $('#first').append('<b>buga</b>');
@@ -562 +583 @@
     reset();
     $("#sap").append(document.getElementById('form'));
-    ok( $("#sap>form").size() == 1, "Check for appending a form" );  // Bug #910
+    ok( $("#sap>form").size() == 1, "Check for appending a form" ); // Bug #910
 
     reset();
@@ -598 +619 @@
     
     t( "Append Select", "#appendSelect1, #appendSelect2", ["appendSelect1", "appendSelect2"] );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    var d = $("<div/>").appendTo("#nonnodes").append(j);
+    equals( $("#nonnodes").length, 1, "Check node,textnode,comment append moved leaving just the div" );
+    ok( d.contents().length >= 2, "Check node,textnode,comment append works" );
+    d.contents().appendTo("#nonnodes");
+    d.remove();
+    ok( $("#nonnodes").contents().length >= 2, "Check node,textnode,comment append cleanup worked" );
 });
 
@@ -826 +856 @@
 
 test("find(String)", function() {
-    expect(1);
+    expect(2);
     ok( 'Yahoo' == $('#foo').find('.blogTest').text(), 'Check for find' );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    equals( j.find("div").length, 0, "Check node,textnode,comment to find zero divs" );
 });
 
 test("clone()", function() {
-    expect(3);
+    expect(4);
     ok( 'This is a normal link: Yahoo' == $('#en').text(), 'Assert text for #en' );
     var clone = $('#yahoo').clone();
     ok( 'Try them out:Yahoo' == $('#first').append(clone).text(), 'Check for clone' );
     ok( 'This is a normal link: Yahoo' == $('#en').text(), 'Reassert text for #en' );
+    // using contents will get comments regular, text, and comment nodes
+    var cl = $("#nonnodes").contents().clone();
+    ok( cl.length >= 2, "Check node,textnode,comment clone works (some browsers delete comments on clone)" );
 });
 
@@ -874 +911 @@
 
     var settings = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
-        options =     { xnumber2: 1, xstring2: "x", xxx: "newstring" },
+        options = { xnumber2: 1, xstring2: "x", xxx: "newstring" },
         optionsCopy = { xnumber2: 1, xstring2: "x", xxx: "newstring" },
         merged = { xnumber1: 5, xnumber2: 1, xstring1: "peter", xstring2: "x", xxx: "newstring" },
@@ -920 +957 @@
     var defaults = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
         defaultsCopy = { xnumber1: 5, xnumber2: 7, xstring1: "peter", xstring2: "pan" },
-        options1 =     { xnumber2: 1, xstring2: "x" },
+        options1 = { xnumber2: 1, xstring2: "x" },
         options1Copy = { xnumber2: 1, xstring2: "x" },
-        options2 =     { xstring2: "xx", xxx: "newstringx" },
+        options2 = { xstring2: "xx", xxx: "newstringx" },
         options2Copy = { xstring2: "xx", xxx: "newstringx" },
         merged2 = { xnumber1: 5, xnumber2: 1, xstring1: "peter", xstring2: "xx", xxx: "newstringx" };
@@ -942 +979 @@
 
 test("val(String)", function() {
-    expect(3);
+    expect(4);
     document.getElementById('text1').value = "bla";
     ok( $("#text1").val() == "bla", "Check for modified value of input element" );
@@ -950 +987 @@
     $("#select1").val("3");
     ok( $("#select1").val() == "3", "Check for modified (via val(String)) value of select element" );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.val("asdf");
+    equals( j.val(), "asdf", "Check node,textnode,comment with val()" );
+    j.removeAttr("value");
 });
 
@@ -955 +998 @@
 
 test("html(String)", function() {
-    expect(10);
+    expect(11);
     var div = $("#main > div");
     div.html("<b>test</b>");
@@ -964 +1007 @@
     ok( pass, "Set HTML" );
 
+    reset();
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.html("<b>bold</b>");
+    equals( j.html().toLowerCase(), "<b>bold</b>", "Check node,textnode,comment with html()" );
+
     $("#main").html("<select/>");
     $("#main select").html("<option>O1</option><option selected='selected'>O2</option><option>O3</option>");
@@ -980 +1029 @@
 
 test("filter()", function() {
-    expect(4);
+    expect(6);
     isSet( $("#form input").filter(":checked").get(), q("radio2", "check1"), "filter(String)" );
     isSet( $("p").filter("#ap, #sndp").get(), q("ap", "sndp"), "filter('String, String')" );
     isSet( $("p").filter("#ap,#sndp").get(), q("ap", "sndp"), "filter('String,String')" );
     isSet( $("p").filter(function() { return !$("a", this).length }).get(), q("sndp", "first"), "filter(Function)" );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    equals( j.filter("span").length, 1, "Check node,textnode,comment to filter the one span" );
+    equals( j.filter("[name]").length, 0, "Check node,textnode,comment to filter the one span" );
 });
 
@@ -1056 +1110 @@
     var pass = true, div = $("div");
     div.show().each(function(){
-      if ( this.style.display == "none" ) pass = false;
+        if ( this.style.display == "none" ) pass = false;
     });
     ok( pass, "Show" );
@@ -1062 +1116 @@
 
 test("addClass(String)", function() {
-    expect(1);
+    expect(2);
     var div = $("div");
     div.addClass("test");
@@ -1070 +1124 @@
     }
     ok( pass, "Add Class" );
+
+    // using contents will get regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.addClass("asdf");
+    ok( j.hasClass("asdf"), "Check node,textnode,comment for addClass" );
 });
 
 test("removeClass(String) - simple", function() {
-    expect(3);
+    expect(4);
     var div = $("div").addClass("test").removeClass("test"),
         pass = true;
@@ -1094 +1153 @@
     ok( div.is('.test'), "Empty string passed to removeClass" );
     
+    // using contents will get regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.removeClass("asdf");
+    ok( !j.hasClass("asdf"), "Check node,textnode,comment for removeClass" );
 });
 
@@ -1112 +1175 @@
 
 test("text(String)", function() {
-    expect(1);
+    expect(4);
     ok( $("#foo").text("<div><b>Hello</b> cruel world!</div>")[0].innerHTML == "&lt;div&gt;&lt;b&gt;Hello&lt;/b&gt; cruel world!&lt;/div&gt;", "Check escaped text" );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.text("hi!");
+    equals( $(j[0]).text(), "hi!", "Check node,textnode,comment with text()" );
+    equals( j[1].nodeValue, " there ", "Check node,textnode,comment with text()" );
+    equals( j[2].nodeType, 8, "Check node,textnode,comment with text()" );
 });
 
@@ -1156 +1226 @@
 
 test("remove()", function() {
-    expect(4);
+    expect(6);
     $("#ap").children().remove();
     ok( $("#ap").text().length > 10, "Check text is not removed" );
@@ -1165 +1235 @@
     ok( $("#ap").text().length > 10, "Check text is not removed" );
     ok( $("#ap").children().length == 1, "Check filtered remove" );
+
+    // using contents will get comments regular, text, and comment nodes
+    equals( $("#nonnodes").contents().length, 3, "Check node,textnode,comment remove works" );
+    $("#nonnodes").contents().remove();
+    equals( $("#nonnodes").contents().length, 0, "Check node,textnode,comment remove works" );
 });
 
 test("empty()", function() {
-    expect(2);
+    expect(3);
     ok( $("#ap").children().empty().text().length == 0, "Check text is removed" );
     ok( $("#ap").children().length == 4, "Check elements are not removed" );
+
+    // using contents will get comments regular, text, and comment nodes
+    var j = $("#nonnodes").contents();
+    j.empty();
+    equals( j.html(), "", "Check node,textnode,comment empty works" );
 });
 
@@ -1204 +1284 @@
 
 test("contents()", function() {
-    expect(10);
+    expect(12);
     equals( $("#ap").contents().length, 9, "Check element contents" );
     ok( $("#iframe").contents()[0], "Check existance of IFrame document" );
@@ -1228 +1308 @@
     $("table", ibody).remove();
     equals( $("div", ibody).length, 1, "Check for JS error on add and delete of a table in IFrame" );
-});
+
+    // using contents will get comments regular, text, and comment nodes
+    var c = $("#nonnodes").contents().contents();
+    equals( c.length, 1, "Check node,textnode,comment contents is just one" );
+    equals( c[0].nodeValue, "hi", "Check node,textnode,comment contents is just the one from span" );
+});

trunk/jquery/test/unit/event.js

@@ -2 +2 @@
 
 test("bind()", function() {
-    expect(15);
+    expect(16);
 
     var handler = function(event) {
@@ -68 +68 @@
     // Trigger the remaining fn (1)
     $("#firstp").trigger("click");
+
+    // using contents will get comments regular, text, and comment nodes
+    $("#nonnodes").contents().bind("tester", function () {
+        equals(this.nodeType, 1, "Check node,textnode,comment bind just does real nodes" );
+    }).trigger("tester");
 });
 

trunk/jquery/test/unit/fx.js

@@ -2 +2 @@
 
 test("animate(Hash, Object, Function)", function() {
-    expect(1);
+    expect(3);
     stop();
     var hash = {opacity: 'show'};
@@ -8 +8 @@
     $('#foo').animate(hash, 0, function() {
         ok( hash.opacity == hashCopy.opacity, 'Check if animate changed the hash parameter' );
+    });
+    // using contents will get comments regular, text, and comment nodes
+    $("#nonnodes").contents().animate({paddingLeft:"5px"}, 100, function () {
+        equals(this.nodeType, 1, "Check node,textnode,comment animate just does real nodes" );
+        equals($(this).css("paddingLeft"), "5px", "Check node,textnode,comment animate just does real nodes" );
         start();
     });