Bug Tracker

Ticket #14179 (closed bug: fixed)

Opened 12 months ago

Last modified 5 months ago

$.isNumeric returns true if argument is an array that contains one integer

Reported by: nir@… Owned by: dmethvin
Priority: low Milestone: 1.11.1/2.1.1
Component: core Version: 2.0.3
Keywords: Cc:
Blocking: Blocked by:

Description

If an argument is an array that contains one value that is numeric, for example [1], $.isNumeric([1]) will return true which is incorrect.

Change History

comment:1 Changed 12 months ago by rwaldron

  • Status changed from new to closed
  • Resolution set to notabug

isNumeric uses parseFloat ( http://es5.github.io/#x15.1.2.3), of which the spec's first operation is ToString(string) (because parseFloat assumes the input to be a string). Array.prototype.toString is effectively a "join" ( http://es5.github.io/#x15.4.4.2):

([1].toString() === "1") === ((parseFloat([1]) === 1) === (parseFloat("1") === 1))

comment:2 follow-up: ↓ 4 Changed 12 months ago by nir@…

Hi rwaldron,

It seems that you're focused on why this bug occurs and not why it should be fixed. As I mentioned, the expected result of using isNumeric on an array is false and this bug can cause plenty of issues for developers using jQuery.

For example, let's say I have two arguments, first is [4] (an array with one item that contains the number 4) and the second is 3 (the number 3) and I want to sum the two arguments in case both of them are numeric, isNumeric will return true on both and I'll get the string 43 instead of the number 7.

Nir

comment:3 Changed 12 months ago by scott.gonzalez

rwaldron's response may seem strange, but your use case isn't valid anyway. You still want "4" (the string containing the character 4) to return true, right? So add( "4", 3 ) would result in "43" as well. If you want to perform math, you'll need to parseFloat() anyway, and parseFloat([ 4 ]) + parseFloat( 3 ) will result in the correctly value of 7, which is what rwaldron was trying to point out.

The question is whether we should consider an array to ever be numeric. If we're going to say that this is not a bug, we should probably update the documentation.

comment:4 in reply to: ↑ 2 Changed 12 months ago by rwaldron

Replying to nir@…:

Hi rwaldron,

It seems that you're focused on why this bug occurs and not why it should be fixed. As I mentioned, the expected result of using isNumeric on an array is false and this bug can cause plenty of issues for developers using jQuery.

For example, let's say I have two arguments, first is [4] (an array with one item that contains the number 4) and the second is 3 (the number 3) and I want to sum the two arguments in case both of them are numeric, isNumeric will return true on both and I'll get the string 43 instead of the number 7.

Nir

If your program is expecting an array with a single item that will be used in a summing operation, I would recommend that your program be responsible for extracting the value from the array.

comment:5 Changed 12 months ago by nir@…

You're focusing on why this happens and on how I should change my code and not on the fact that isNumeric doesn't handle well with arrays and returns true when the expected result is false. The example I mentioned early is made up and I did ended up adding a function that checks my data that doesn't rely on isNumeric so I do have a solution, but still thinks this should be fixed or mentioned in the documentation so developers using jQuery will know about this.

Thanks :)

comment:6 Changed 12 months ago by rwaldron

  • Status changed from closed to reopened
  • Resolution notabug deleted

comment:7 Changed 12 months ago by rwaldron

  • Owner set to rwaldron
  • Status changed from reopened to assigned

You win, patch for your use case comin' right up.

comment:8 Changed 12 months ago by dmethvin

I agree that jQuery.isNumeric([1]) being valid seems odd, but what exactly is the fix? Are we going to check explicitly for Array with one element, or is there some more drastic road block we'll put up against objects with .toString() values we don't like? I'd still want this to work:  http://jsfiddle.net/aqGmj/

comment:9 Changed 12 months ago by rwaldron

Dave, my patch (still local) is literally:

if ( jQuery.isArray(obj) ) { 
    return false; 
}

;)

comment:10 Changed 12 months ago by rwaldron

Just kidding... that isn't very jQuery-ish...

return !jQuery.isArray(obj) && !isNaN( parseFloat(obj) ) && isFinite( obj );

comment:11 Changed 12 months ago by jaubourg

Wouldn't document that it "returns true if the string representation of the argument can be parsed as a number" be enough?

comment:12 Changed 12 months ago by dmethvin

@jaubourg, that would be clarifying the current behavior in documentation. So, ["42"].toString() ==> "42" which is a number. No code change required.

@nir, what is your goal in calling jQuery.isNumeric() here? How do you get into a situation of passing it an array with one numeric element? Are you just looking for a small subset of cases like primitive 42 and "42", or are you attempting to also filter out JavaScript objects that have a .toString() returning numbers?

comment:13 Changed 11 months ago by timmywil

  • Priority changed from undecided to low
  • Component changed from unfiled to core

comment:14 Changed 5 months ago by dmethvin

  • Milestone changed from None to 1.11.1/2.1.1

comment:15 Changed 5 months ago by dmethvin

  • Owner changed from rwaldron to dmethvin

comment:16 Changed 5 months ago by gibson042

Having not heard back from the reporter, I'd like to wontfix this one. jQuery.isNumeric does exactly what we need it to.

comment:17 Changed 5 months ago by dmethvin

OP had me convinced that $.isNumeric([42]) === true is wrong, and the .isArray() check fixes it. The more general .toString() cases still work.

comment:18 Changed 5 months ago by Dave Methvin

  • Status changed from assigned to closed
  • Resolution set to fixed

Core: Arrays like [42] should fail .isNumeric()

Fixes #14179

Changeset: 10efa1f5b44046aab6bcc8423322a41923faa290

Note: See TracTickets for help on using tickets.