Issue with time_select view helper defaulting to year 1

[Andrew Kaspick]

Currently the time_select helper method defaults to year 1 as seen at https://github.com/rails/rails/blob/master/actionview/lib/action_view/helpers/date_helper.rb#L832

This appears to be an issue when creating new Time values.

When assigning the multi-parameter values  (time(1i), time(2i), etc) for the time, the code essentially creates a new time value with Time.new(1,1,1,18,0,0) using 1 as the year.

This is the result I get on my servers running Ubuntu using year 1...

irb(main):001:0> time = Time.new(1,1,1,18,0,0)
=> 0001-01-01 18:00:00 +0000
irb(main):002:0> TZInfo::Timezone.get("America/Chicago").local_to_utc(time)
=> 0001-01-01 23:50:36 UTC

Note that the time when using the time zone is off by 5 hours and 50 minutes which appears to come from the LMT zone (local mean time) (no time zones existed in year 1?)

But if I run the same code in my local dev enironment (a mac), I get the following...

rb(main):001:0> time = Time.new(1,1,1,18,0,0)
=> 0001-01-01 18:00:00 -0500
irb(main):002:0> TZInfo::Timezone.get("America/Chicago").local_to_utc(time)
=> 0001-01-02 00:00:00 UTC

This results in the time using the correct time zone from my app.

If I use 2017 (current year) instead of 1, then both my local and Ubuntu servers give me the same correct results.

So my question is, should the current year be used as the default instead of 1 (which appears to be problematic)?

Andrew

[Andrew Kaspick]

Any comments on why year 1 is used as the default?  This appears to have been made as the choice 9 or so years ago.  It's an easy change, but it'd be nice to hear a reason why 1 has been used as the default before I open a new PR.

[Matthew Draper]

> Any comments on why year 1 is used as the default? This appears to have been made as the choice 9 or so years ago. It's an easy change, but it'd be nice to hear a reason why 1 has been used as the default before I open a new PR.

It looks like it was https://github.com/rails/rails/pull/4641


From context, I surmise that only date_select was particularly considered, which would explain why it didn’t really matter what the default was, beyond the fact it existed.



My possible concerns around changing this are:

* existing callers, especially of a (more common?) year-ignoring date_select, may be relying on the current value

* making the default a value that changes over time seems to risk future surprises


Tying in with the latter, I’d generally advise against doing TZ math on a datetime whose year you don’t know / is undefined. If your use case means you know it’s definitely the current year, you probably want to be explicit about that, and not rely on a default anyway.

[Andrew Kaspick]

Unfortunately you can't set an explicit year to be used with a time_select (when the time is nil).

The issue is that TZ math IS occurring already implicitly in Rails core when creating a new time object without a timezone.  When the year defaults to 1 my times are being offset by 5 hours and 50 minutes automatically because it's using LMT as a zone which I assume is because year 1 has no time zones.  As mentioend in my original message, this is not an issue on my mac (it uses the apps correct default timezone), but on Ubuntu it's using LMT as a zone in year 1.

When the time value is nil, there needs to be a way to set the default year.  Perhaps a PR is required for setting an explicit default if "1" is considered an acceptable default currently?

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.