From ffbedf0262b3eea906f39c0115b818d7456a3994 Mon Sep 17 00:00:00 2001 From: David Serduke Date: Fri, 7 Dec 2007 01:52:21 +0000 Subject: [PATCH] Fixed #1039 and #1733 by going through the core API and making them text node and comment node safe. --- src/core.js | 29 ++++++--- src/event.js | 15 ++++- src/fx.js | 6 ++ src/selector.js | 6 +- test/index.html | 1 + test/unit/core.js | 181 ++++++++++++++++++++++++++++++++++++++-------------- test/unit/event.js | 7 +- test/unit/fx.js | 7 +- 8 files changed, 188 insertions(+), 64 deletions(-) diff --git a/src/core.js b/src/core.js index be989ea..313cdfa 100644 --- a/src/core.js +++ b/src/core.js @@ -251,13 +251,15 @@ jQuery.fn = jQuery.prototype = { append: function() { return this.domManip(arguments, true, false, function(elem){ - this.appendChild( elem ); + if (this.nodeType == 1) + this.appendChild( elem ); }); }, prepend: function() { return this.domManip(arguments, true, true, function(elem){ - this.insertBefore( elem, this.firstChild ); + if (this.nodeType == 1) + this.insertBefore( elem, this.firstChild ); }); }, @@ -402,6 +404,9 @@ jQuery.fn = jQuery.prototype = { } 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 || jQuery.inArray(this.name, value) >= 0); @@ -722,18 +727,19 @@ jQuery.extend({ // internal only, use addClass("class") 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; }); }, // 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(" ") : + ""; }, // internal only, use is(".class") @@ -1014,6 +1020,10 @@ jQuery.extend({ }, 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 ) ? {} : jQuery.props; @@ -1267,7 +1277,8 @@ jQuery.each({ jQuery.each({ removeAttr: function( name ) { jQuery.attr( this, name, "" ); - this.removeAttribute( name ); + if (this.nodeType == 1) + this.removeAttribute( name ); }, addClass: function( classNames ) { diff --git a/src/event.js b/src/event.js index 7c718f4..f71a1df 100644 --- a/src/event.js +++ b/src/event.js @@ -8,6 +8,9 @@ jQuery.event = { // Bind an event to an element // 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 if ( jQuery.browser.msie && element.setInterval != undefined ) @@ -83,6 +86,10 @@ jQuery.event = { // 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; // Namespaced event handlers @@ -147,6 +154,10 @@ jQuery.event = { // 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 event = !data[0] || !data[0].preventDefault; @@ -273,7 +284,7 @@ jQuery.event = { if ( event.pageX == null && event.clientX != null ) { 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); } // Add which for key events @@ -437,7 +448,7 @@ function bindReady(){ // 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 // http://javascript.nwbox.com/IEContentLoaded/ diff --git a/src/fx.js b/src/fx.js index b35dbcd..52225c8 100644 --- a/src/fx.js +++ b/src/fx.js @@ -69,6 +69,9 @@ jQuery.fn.extend({ var optall = jQuery.speed(speed, easing, callback); 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; @@ -135,6 +138,9 @@ jQuery.fn.extend({ return queue( this[0], type ); return this.each(function(){ + if ( this.nodeType != 1) + return; + if ( fn.constructor == Array ) queue(this, type, fn); else { diff --git a/src/selector.js b/src/selector.js index da27245..c35fbb1 100644 --- a/src/selector.js +++ b/src/selector.js @@ -96,9 +96,9 @@ jQuery.extend({ if ( typeof t != "string" ) 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) context = context || document; diff --git a/test/index.html b/test/index.html index 4f68639..cdd6a30 100644 --- a/test/index.html +++ b/test/index.html @@ -171,6 +171,7 @@ Z +
hi there
diff --git a/test/unit/core.js b/test/unit/core.js index bc1def3..7ba7eb5 100644 --- a/test/unit/core.js +++ b/test/unit/core.js @@ -141,18 +141,18 @@ test("isFunction", function() { ok( jQuery.isFunction(fn), "Recursive Function Call" ); - fn({ some: "data" }); + fn({ some: "data" }); }; callme(function(){ - callme(function(){}); + callme(function(){}); }); }); var foo = false; test("$('html')", function() { - expect(4); + expect(5); reset(); foo = false; @@ -166,6 +166,9 @@ test("$('html')", function() { ok( $("")[0], "Creating a link" ); reset(); + + var j = $("hi there "); + ok( j.length >= 2, "Check node,textnode,comment creation (some browsers delete comments)" ); }); test("$('html', context)", function() { @@ -232,7 +235,7 @@ test("each(Function)", function() { div.each(function(){this.foo = 'zoo';}); 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" ); }); @@ -277,15 +280,15 @@ test("attr(String)", function() { }); 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(); + }); + }); } test("attr(String, Function)", function() { @@ -298,18 +301,18 @@ test("attr(Hash)", function() { expect(1); 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" ); }); 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" ); @@ -338,6 +341,13 @@ test("attr(String, Object)", function() { $("#name").attr('someAttr', 1); 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(); var type = $("#check2").attr('type'); @@ -362,19 +372,19 @@ test("attr(String, Object)", function() { }); 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(); + }); + }); } test("css(String|Hash)", function() { @@ -408,7 +418,7 @@ test("css(String|Hash)", function() { }); test("css(String, Object)", function() { - expect(19); + expect(20); ok( $('#foo').is(':visible'), 'Modifying CSS display: Assert element is visible'); $('#foo').css('display', 'none'); ok( !$('#foo').is(':visible'), 'Modified CSS display: Assert element is hidden'); @@ -437,6 +447,11 @@ test("css(String, Object)", function() { $('#foo').css("filter", "progid:DXImageTransform.Microsoft.Chroma(color='red');"); } 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" ); }); test("jQuery.css(elem, 'height') doesn't clear radio buttons (bug #1095)", function () { @@ -467,7 +482,7 @@ test("text()", function() { }); test("wrap(String|Element)", function() { - expect(6); + expect(8); var defaultText = 'Try them out:' var result = $('#first').wrap('
').text(); ok( defaultText == result, 'Check for wrapping of on-the-fly html' ); @@ -486,6 +501,12 @@ test("wrap(String|Element)", function() { $(checkbox).wrap( '' ); 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(""); + 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" ); }); test("wrapAll(String|Element)", function() { @@ -525,7 +546,7 @@ test("wrapInner(String|Element)", function() { }); test("append(String|Element|Array<Element>|jQuery)", function() { - expect(18); + expect(21); var defaultText = 'Try them out:' var result = $('#first').append('buga'); ok( result.text() == defaultText + 'buga', 'Check if text appending works' ); @@ -561,7 +582,7 @@ test("append(String|Element|Array<Element>|jQuery)", function() { 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(); var pass = true; @@ -597,6 +618,15 @@ test("append(String|Element|Array<Element>|jQuery)", function() { .append(''); t( "Append Select", "#appendSelect1, #appendSelect2", ["appendSelect1", "appendSelect2"] ); + + // using contents will get comments regular, text, and comment nodes + var j = $("#nonnodes").contents(); + var d = $("
").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" ); }); test("appendTo(String|Element|Array<Element>|jQuery)", function() { @@ -825,16 +855,23 @@ test("end()", function() { }); 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)" ); }); test("is(String)", function() { @@ -873,7 +910,7 @@ test("$.extend(Object, Object)", function() { expect(17); 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" }, deep1 = { foo: { bar: true } }, @@ -919,9 +956,9 @@ test("$.extend(Object, Object)", function() { 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" }; @@ -941,7 +978,7 @@ test("val()", function() { }); test("val(String)", function() { - expect(3); + expect(4); document.getElementById('text1').value = "bla"; ok( $("#text1").val() == "bla", "Check for modified value of input element" ); $("#text1").val('test'); @@ -949,12 +986,18 @@ test("val(String)", function() { $("#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"); }); var scriptorder = 0; test("html(String)", function() { - expect(10); + expect(11); var div = $("#main > div"); div.html("test"); var pass = true; @@ -963,6 +1006,12 @@ test("html(String)", function() { } ok( pass, "Set HTML" ); + reset(); + // using contents will get comments regular, text, and comment nodes + var j = $("#nonnodes").contents(); + j.html("bold"); + equals( j.html().toLowerCase(), "bold", "Check node,textnode,comment with html()" ); + $("#main").html("