Skip to main content

Bug Tracker

Side navigation

#1733 closed bug (fixed)

Opened September 26, 2007 09:11AM UTC

Closed December 07, 2007 01:55AM UTC

Last modified October 14, 2008 10:22AM UTC

addClass gives error on textnodes

Reported by: achun Owned by:
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: addClass Cc:
Blocked by: Blocking:
Description
$("<b>addClass</b>\\n<b>addClass</b>").addClass('tmp')

error!

in className:add fun

		add: function( elem, c ){
			jQuery.each( (c || "").split(/\\s+/), function(i, cur){
				if ( !jQuery.className.has( elem.className, cur ) )
					elem.className += ( elem.className ? " " : "" ) + cur;
			});
		},

fix?

		add: function( elem, c ){
			jQuery.each( (c || "").split(/\\s+/), function(i, cur){
				if ( !jQuery.className.has( elem.className||"", cur ) )
					elem.className += ( elem.className ? " " : "" ) + cur;
			});
		},
Attachments (1)
  • 1733.diff (2.5 KB) - added by brandon September 27, 2007 03:11PM UTC.

    Patch + Tests for text nodes in jQuery.className methods

Change History (7)

Changed September 27, 2007 03:12PM UTC by brandon comment:1

need: ReviewCommit

I've attached patch + tests to make jQuery.className.remove/add/has not fail when it gets a non element node.

However, I'm wondering if the better solution (or in addition to) should be that text nodes do not make it into the jQuery collection via jQuery.clean.

Changed November 17, 2007 09:29PM UTC by davidserduke comment:2

I don't think we can keep text nodes out of the jQuery collection. I looked in to that recently and realized there are several ways to get text nodes in. In fact, the function .contents() specifically includes text nodes.

http://docs.jquery.com/Traversing/contents

With that said, do you think it makes sense to implement the patch? It looks like a good solution to me.

Changed November 18, 2007 05:06PM UTC by brandon comment:3

Replying to [comment:2 davidserduke]:

Yeah we definitely can't keep text nodes out now ... however we need to be mindful of this in our manipulation methods such as css, attr and the class methods. We should probably go ahead and be proactive and test/patch the other manipulation methods.

Changed December 06, 2007 06:04PM UTC by davidserduke comment:4

summary: addClass?addClass fails on textnodes

Changed December 06, 2007 06:05PM UTC by davidserduke comment:5

summary: addClass fails on textnodesaddClass gives error on textnodes

Changed December 06, 2007 06:06PM UTC by davidserduke comment:6

See #1039 for a similar ticket.

Changed December 07, 2007 01:55AM UTC by davidserduke comment:7

resolution: → fixed
status: newclosed

Fixed in [4062] taking Brandon's suggestion that text nodes should not break jQuery even when they are in the jQuery object. addClass was fixed along with the other functions in the API.