#1733 closed bug (fixed)
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)
Change History (8)
Changed 15 years ago by
comment:1 Changed 15 years ago by
need: | Review → Commit |
---|
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.
comment:2 follow-up: 3 Changed 15 years ago by
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.
comment:3 Changed 15 years ago by
Replying to 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.
comment:4 Changed 15 years ago by
Summary: | addClass? → addClass fails on textnodes |
---|
comment:5 Changed 15 years ago by
Summary: | addClass fails on textnodes → addClass gives error on textnodes |
---|
comment:7 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
Patch + Tests for text nodes in jQuery.className methods