Bug 5280 – Date.setMonth fails with negative values

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Maciej Stachowiak]

Steps to reproduce:

1) Go to http://www.rte.ie/news/nuacht.html

Results:

hang

[Maciej Stachowiak]

Also filed as <rdar://problem/4092064> hanging loading page; rte.ie (works in IE and Firefox)

Created attachment 4252 [details]
testcase

It's got something to do with dates, but is it a bug in JSC's date
implementation? Investigating...

Created attachment 4254 [details]
handle negative month values in kjs::maketime

Created attachment 4255 [details]
regression test

[Darin Adler]

Comment on attachment 4254 [details]
handle negative month values in kjs::maketime

There really doesn't need to be a second branch. One case for negative numbers
and another for >= 0 would have been fine.

That having been said, this does look fine as-is. r=me

[Darin Adler]

I think a better fix is to not use CoreFoundation-based date routines. I have a patch in the works to do 
that.

[Darin Adler]

Created attachment 4262 [details]
remove code to use CFDate, use NaN instead of LONG_MIN for bad dates

This patch has a few changes, but I think it's still fine to land it all at
once:

    1) use the real gmtime, localtime, mktime, and time
    2) some tweaks to styleFromArgString and formatLocaleDate (mainly to use
arg values intead of arg counts)
    3) added a FIXME to some code that looks broken for the non-__APPLE__ case
    4) took out unnecessary defined use (#if !X does work fine if X is not
defined at all!)
    5) changed invalidDate to NaN now that it's used in a context where we have
a double rather than a time_t
    6) removed some code that's now unnecessary if invalidDate is NaN
    7) put the code to use CoreFoundation to do locale-specific dates inside
__APPLE__ rather than APPLE_CHANGES

[Darin Adler]

Forgot to mention: I ran all the tests and everything passes, including the new regression test that Mitz 
attached to this bug.

[Darin Adler]

Created attachment 4263 [details]
alternate version of patch that uses _r versions of functions too (either one seems fine to me)

I think it's fine to use the _r versions of the functions even if they aren't
universally available. It's easy to provide "adapter" implementations for
platforms that lack them even if they aren't threadsafe.

[Darin Adler]

These patches also address bug 5154.

[Maciej Stachowiak]

Comment on attachment 4263 [details]
alternate version of patch that uses _r versions of functions too (either one seems fine to me)

Using the _r versions is better. For platforms that don't have them, we can
solve it on a case-by-case basis.

[Maciej Stachowiak]

Comment on attachment 4262 [details]
remove code to use CFDate, use NaN instead of LONG_MIN for bad dates

Marking this alternate patch r- so it goes away from the review queue.

[Geoffrey Garen]

*** Bug 5154 has been marked as a duplicate of this bug. ***