Bug Tracker

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#9427 closed bug (fixed)

Passing undefined to .text() does not trigger setter

Reported by: Xavi Owned by:
Priority: low Milestone: 1.7.2
Component: manipulation Version: 1.6.1
Keywords: Cc:
Blocked by: Blocking:

Description

Passing undefined to $.fn.text() triggers getter functionality instead of the setter functionality. For example the following code throws an exception:

var text;
$("<div/>").text(text).append($("<span/>"))

Change History (16)

comment:1 Changed 12 years ago by Rick Waldron

Component: unfiledmanipulation
Priority: undecidedlow
Resolution: invalid
Status: newclosed

This is expected behaviour.

comment:3 Changed 12 years ago by Xavi

Why is it expected behavior? It breaks chaining and causes unexpected exceptions.

comment:4 Changed 12 years ago by Rick Waldron

It's expected that the developer knows to test their own variables before passing them to methods that have overloaded signatures.

var text;
$("<div/>").text( text || "" ).append($("<span/>"))

http://jsfiddle.net/rwaldron/sTesj/

The code above will work fine without breaking the existing api.

comment:5 Changed 12 years ago by Rick Waldron

As noted on the pull request: http://api.jquery.com/text/ The documentation specifies the valid input to the method and their expected outcome.

comment:6 Changed 12 years ago by Xavi

FWIW, I would recommend using this work around instead:

var text;
$("<div/>").text( text + "" ).append($("<span/>"));

It handles 0 and null better and saves 1 byte.

Also, FWIW, this bug also affects $.fn.html, $.fn.attr, $.fn.width, $.fn.height, and number other functions.

I replied to http://bugs.jquery.com/ticket/5571, but it might be worth repeating my argument here. Basically, I feel that the getter/setter functionality should be determined by arguments.length instead of the argument values. IMO, this leads to a more consistent API and is less surprising to the user.

There is the argument that undefined is an invalid value, so breaking the chain makes sense since it acts like a form of error reporting. I disagree with this logic. As a form of error reporting, breaking the chain doesn't always work and leads to confusing error messages. (See ticket for more detailed explanation).

comment:7 Changed 12 years ago by Timmy Willison

#9826 is a duplicate of this ticket.

comment:8 Changed 12 years ago by TJ Holowaychuk

I agree that this is a bug, arity > 1 shows the intention

comment:9 Changed 12 years ago by Rick Waldron

#10396 is a duplicate of this ticket.

comment:10 Changed 12 years ago by Timmy Willison

#10671 is a duplicate of this ticket.

comment:11 Changed 12 years ago by Rick Waldron

#10749 is a duplicate of this ticket.

comment:12 Changed 12 years ago by dmethvin

Keywords: 1.8-discuss added
Resolution: invalid
Status: closedreopened

Reopening for 1.8 discussion -- should we have uniform behavior for .method(undefined/null) ?

comment:13 Changed 12 years ago by dmethvin

Status: reopenedopen

comment:14 Changed 11 years ago by dmethvin

Keywords: 1.8-discuss removed
Resolution: fixed
Status: openclosed

comment:15 Changed 11 years ago by dmethvin

Milestone: 1.next1.7.2

comment:16 Changed 11 years ago by DNS

text + ""
text || ""

Note that these two have different behavior in cases other than those you've considered. If text is an empty array, for example, the previous effective value would have been [], while in jQuery 1.7.2 it becomes "[object Object]".

It appears that .text([]) is a no-op in jQuery pre-1.7.2. Some of my code was unknowingly relying on this behavior, and broke when I upgraded to 1.7.2. I'm fixing the code in question, which shouldn't be passing [] to text() in the first place. But I figured you might want to know about this, since it does represent an undocumented change, however minor.

Note: See TracTickets for help on using tickets.