Bug Tracker

Modify

Ticket #10690 (closed bug: fixed)

Opened 2 years ago

Last modified 2 years ago

isNumeric

Reported by: shiroiyamii@… Owned by: dmethvin
Priority: blocker Milestone: 1.7.1
Component: core Version: 1.7
Keywords: Cc:
Blocking: Blocked by:

Description

The isNumeric function use does not give the right result in every case. If the "number" is a date object it returns true, but a date is not a number.

the right functio should be

isNumeric: function( obj ) {
return !isNaN(parseFloat(obj)) && isFinite(obj);
}

Here's a site (not mine) that proves it:
 http://dl.dropbox.com/u/35146/js/tests/isNumber.html

But 'cause it's not my site I copy pasted the site and add the jQuery function
Result:  http://img42.imageshack.us/content_round.php?page=done&l=img42/4489/20111106yami000.png

Change History

comment:1 Changed 2 years ago by rwaldron

  • Owner set to shiroiyamii@…
  • Status changed from new to pending
  • Component changed from unfiled to core

Thanks for taking the time to contribute to the jQuery project! Please provide a complete reduced test case on jsFiddle to help us assess your ticket!

Additionally, be sure to test against the jQuery Edge version to ensure the issue still exists. To get you started, use this boilerplate:  http://jsfiddle.net/FrKyN/ Open the link and click to "Fork" (in the top menu) to get started.

comment:2 Changed 2 years ago by shiroiyamii@…

  • Status changed from pending to new

The problem still exist in the jQuery Edge.

Here is a test case that shows only the date problem
 http://jsfiddle.net/hdswj/1/

and here is a test case that is more detailed
 http://jsfiddle.net/hdswj/2/

comment:3 Changed 2 years ago by rwaldron

On the "more detailed" test, I'm getting all passing tests, is that correct?

comment:4 Changed 2 years ago by dmethvin

  • Owner changed from shiroiyamii@… to dmethvin
  • Priority changed from undecided to blocker
  • Status changed from new to assigned
  • Milestone changed from None to 1.7.1

I'd agree that a Date object isn't logically numeric, although +(new Date) does yield a perfectly usable number and that is essentially what is being measured.

The general case where an object has a .toString() still works for both approaches, and I think we *do* want that. As long as the proposed new code doesn't break that I'm okay with it.

 http://jsfiddle.net/dmethvin/vAPaq/

comment:5 Changed 2 years ago by Dave Methvin

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

Fix #10690. Make sure .isNumeric() can't get a Date.

Also add tests for custom objects with a .toString() method.

Changeset: 795583d2936a48c6f0048e28f74c5fc71e3a60b0

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.