Bug Tracker

Ticket #9427 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

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:
Blocking: Blocked by:

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

comment:1 Changed 4 years ago by rwaldron

  • Priority changed from undecided to low
  • Resolution set to invalid
  • Status changed from new to closed
  • Component changed from unfiled to manipulation

This is expected behaviour.

comment:2 Changed 4 years ago by Xavi

comment:3 Changed 4 years ago by Xavi

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

comment:4 Changed 4 years ago by rwaldron

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 4 years ago by rwaldron

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 4 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 3 years ago by timmywil

#9826 is a duplicate of this ticket.

comment:8 Changed 3 years ago by TJ Holowaychuk

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

comment:9 Changed 3 years ago by rwaldron

#10396 is a duplicate of this ticket.

comment:10 Changed 3 years ago by timmywil

#10671 is a duplicate of this ticket.

comment:11 Changed 3 years ago by rwaldron

#10749 is a duplicate of this ticket.

comment:12 Changed 3 years ago by dmethvin

  • Keywords 1.8-discuss added
  • Status changed from closed to reopened
  • Resolution invalid deleted

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

comment:13 Changed 3 years ago by dmethvin

  • Status changed from reopened to open

comment:14 Changed 3 years ago by dmethvin

  • Keywords 1.8-discuss removed
  • Status changed from open to closed
  • Resolution set to fixed

comment:15 Changed 3 years ago by dmethvin

  • Milestone changed from 1.next to 1.7.2

comment:16 Changed 3 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.