Skip to main content

Bug Tracker

Side navigation

#14179 closed bug (fixed)

Opened July 24, 2013 12:50PM UTC

Closed March 05, 2014 02:50AM UTC

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

Reported by: nir@nirpeled.com Owned by: dmethvin
Priority: low Milestone: 1.11.1/2.1.1
Component: core Version: 2.0.3
Keywords: Cc:
Blocked by: Blocking:
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.

Attachments (0)
Change History (18)

Changed July 24, 2013 03:54PM UTC by rwaldron comment:1

resolution: → notabug
status: newclosed

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))

Changed July 24, 2013 04:17PM UTC by nir@nirpeled.com comment:2

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

Changed July 24, 2013 04:25PM UTC by scottgonzalez comment:3

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.

Changed July 24, 2013 04:37PM UTC by rwaldron comment:4

Replying to [comment:2 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.

Changed July 24, 2013 04:55PM UTC by nir@nirpeled.com comment:5

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 :)

Changed July 24, 2013 05:05PM UTC by rwaldron comment:6

resolution: notabug
status: closedreopened

Changed July 24, 2013 05:08PM UTC by rwaldron comment:7

owner: → rwaldron
status: reopenedassigned

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

Changed July 25, 2013 02:44PM UTC by dmethvin comment:8

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/

Changed July 25, 2013 06:55PM UTC by rwaldron comment:9

Dave, my patch (still local) is literally:

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

;)

Changed July 25, 2013 06:56PM UTC by rwaldron comment:10

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

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

Changed July 25, 2013 08:12PM UTC by jaubourg comment:11

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

Changed July 25, 2013 08:24PM UTC by dmethvin comment:12

@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?

Changed August 13, 2013 02:08PM UTC by timmywil comment:13

component: unfiledcore
priority: undecidedlow

Changed March 03, 2014 04:10PM UTC by dmethvin comment:14

milestone: None1.11.1/2.1.1

Changed March 04, 2014 03:19AM UTC by dmethvin comment:15

owner: rwaldrondmethvin

Changed March 04, 2014 08:21PM UTC by gibson042 comment:16

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

Changed March 04, 2014 08:46PM UTC by dmethvin comment:17

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

Changed March 05, 2014 02:50AM UTC by Dave Methvin comment:18

resolution: → fixed
status: assignedclosed

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

Fixes #14179

Changeset: 10efa1f5b44046aab6bcc8423322a41923faa290