JosephMajaseSithole / datejs

Automatically exported from code.google.com/p/datejs
0 stars 0 forks source link

[FIXED/FUTURE] isDST() broken #23

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
*******************************************************

This issue originated as post in the Datejs forums. See
http://tinyurl.com/2sznas

The issue has been moved here to track it's status. 

*******************************************************

Using version 1.0 Alpha-1 the following code:

  alert( new Date().isDST() );

gives the following error in the Firebug console:

 this.toString().match(/(E|C|M|P)(S|D)T/) has no properties

The above code seems to assume that Date.toString() returns a string
that includes a time zone, and that it matches the regular
expression.

Looking at that particular function:

  Date.prototype.isDST = function () {
    console.log('isDST');
    /* TODO: not sure if this is portable ... get from
Date.CultureInfo? */
    return this.toString().match(/(E|C|M|P)(S|D)T/)[2] == "D";
  };

The value returned by match(...) should be tested to see if a second
element exists before attempting the evaluation.  It also seems to be
a very weak test of whether or not daylight saving is in force (or at
least whether the system executing the code thinks daylight saving is
being observed).

The notes in the code show doubt about the usefulness of the function
- does the CultureInfo section need to define every time zone in the
world, and identify which ones are DST and which aren't?

There is a useful list of time zone abbreviations here:

<URL; http://www.worldtimezone.com/wtz-names/timezonenames.html >

but I have no idea how authoritative they are - as far as I know,
there is no standard for them.

-- 
Rob 

Original issue reported on code.google.com by geoff%co...@gtempaccount.com on 19 Dec 2007 at 7:56

GoogleCodeExporter commented 8 years ago
Funny you should mention this bug, yesterday afternoon we (Dan and I) had a 
two+ hour
conversation directly related to that very function. We did not notice nor
anticipated the exception being thrown, and that's an easy fix to make, but the 
root
of the problem is much more complicated... supporting the functionality across 
all
Timezones globally.

I'm not entirely sure why/how the isDST function made it into core.js anyways. 
Best I
can tell, we don't need it, we're not using it anywhere and obviously it was
originally built as just a stub.

There are several major problems with the JavaScript implementation of DST and
Timezones. Some being critical bugs that have gone unnoticed since JavaScript's
inception.

Combine those problems with the performance problems of the native Date object 
and
we're being forced down the road of creating a whole new implementation of the 
Date
object. That's not something I'm particularly looking forward to. There's been 
some
discussion around this topic in the datejs forums 
(http://www.datejs.com/forums/,
search for "Date2" if you're interested.

At the moment my opinion is the isDST() function will have to be moved into our
future Date2 implementation. That would mean removing the function completely 
from
the current Datejs core.

I will publish a fix to solve the exception, and thankfully the function can 
stand on
it's own, so removing from the core is less of an issue.

If we completely remove from the master source, anyone currently using isDST 
can add
the patched function to their own library or manually tack it onto their 
version of
date.js.

That is not a final decision. We have to discuss some more and I need to sleep 
on the
decision before committing. 

Original comment by geoff%co...@gtempaccount.com on 19 Dec 2007 at 8:03

GoogleCodeExporter commented 8 years ago
Changed the Title to "isDST() broken".

It was improperly named "isDate() broken".

Original comment by geoff%co...@gtempaccount.com on 20 Dec 2007 at 12:15

GoogleCodeExporter commented 8 years ago
We are removing the .isDST() function from the library. At least for the short 
term.
There are several other related functions we would like to add, although I 
think we
might group them all (including .isDST) into an optional module.

I'm marking this issue as [FIXED], although that's not exactly the best 
description.
Basically we're just going to implement and expand on the function in a later 
build.

Original comment by geoff%co...@gtempaccount.com on 20 Dec 2007 at 12:20

GoogleCodeExporter commented 8 years ago
For posterity, here's the last version of .isDST().

/**
 * Determine whether Daylight Saving Time (DST) is in effect
 * @return {Boolean} True if DST is in effect.
 */
$P.isDST = function () {
    /* TODO: not sure if this is portable ... get from Date.CultureInfo? */
    return this.toString().match(/(E|C|M|P)(S|D)T/)[2] == "D";
};

Original comment by geoff%co...@gtempaccount.com on 20 Dec 2007 at 12:22