mde / timezone-js

DEPRECATED: Timezone-enabled JavaScript Date object. Uses Olson zoneinfo files for timezone data.
824 stars 182 forks source link

error on Indian timezones #8

Closed wrodenbusch closed 12 years ago

wrodenbusch commented 12 years ago
    var findApplicableRules = function( year, ruleset )
    {
      var applicableRules = [];

      for (var i = 0; i < ruleset.length; i++)
      {

should be

    var findApplicableRules = function( year, ruleset )
    {
      var applicableRules = [];

      for (var i = 0; ruleset && i < ruleset.length; i++)
      {

because otherwise I get an an error on Indian timezones.

mde commented 12 years ago

Can you give me a better idea of what is breaking here? I can't make a change like this without knowing what it's fixing, and being sure it won't cause problems for other people.

wrodenbusch commented 12 years ago

ruleset is undefined and then it executes ruleset.length which causes a type error. I am assuming that the ruleset is undefined because India does not observe dst or something like that? I have already made the change in production and it is live here: www.sametimeas.com if you want to test it. It does not seem to have negative effects on other timezones. I do not know the internals to be sure this fix is safe but I thought I would post the fix here anyhow in case others have the same issue.

mde commented 12 years ago

Thanks for the context. And I had totally mis-read your fix. Yes, guarding against the ruleset not existing is an obvious right thing to do here. Do you want to do a pull request, so you'll show up as a contributor?

wrodenbusch commented 12 years ago

Sorry. Probably should have just started with a pull request. I just generated a pull request now and for some reason it created another issue. snafu. Thanks for maintaining this library.

mde commented 12 years ago

Thanks for the patch.