Introduce Daylight Saving Time for Drupal

This is a lovely patch, but you do realise that you have opened a can of worms.

Daylight saving is a messy business, I had to write the Contestable Electricity billing for a company here in Australia which had multiple installations in multpile timezones, all with different daylight savings rules, which impacted upon the bill. The way that dst is legistated is very messy and here is Australia it is very messy. Daylight savings don't follow state lines, as some towns that are very close to the border may choose to follow the only the daylight saving period of the next state, and when we had the olympics NSW went into daylight savings months before anyone else. There are other examples of weird things like even the 1 hour forward is not also true. I know that in the 1999 change to daylight savings there was a small set of islands north of New Zealand that went ahead by 2 hours as they wanted to be the first into 2000.

From what I can see of your patch it only allows for a single daylight savings period for the entire system. Their can be situations where there can be users from multiple countries and in different timezones/dst.

I think that entire timezone system needs to looked at again, and maybe re-implement it so that it will allow for dst. Maybe make it a bit more user friendly so instead of setting up the timezone by name much like how operating systems do it, and then this will automatically work out the timezone.

+1 for this is something that would be good, but it still needs work.

The patch works as it is now. I had good reason to limit its capability to the setting where users cannot chose their own timezone. ;)
I also assume that a server administrator knows if he has dst or not. I also didn't (yet) cater for all countries where dst is in use mainly because some insist on not giving a sensible rule for the change dates.

The last comment was mine.

After a code review by Kjartan I provide a slightly improved patch.

I want to stress that solving the DST issue in a "suits everybody" manner isn't the intention of this patch.
It solves the problem for people who have chosen to let all times on their site be displayed in the server's time setting. By covering a large portion of the earth's landmass I hope to be able to help most site admins.

It would be possible to extend the patch to allow users to chose their DST setting, too. In my experience, however, users tend to not care about time settings at all. This may be EUrocentric though, as we have only one time zone. Feedback is welcome.

+1

From a code review I think this is an excellent solution. Could use a little more comments in the section that sets $node->date to explain the (probably good) reasoning behind that chunk of code. I don't have anything resembling a good set of tests so I did not apply the patch and try it.

I just want to point out that this patch still applies and would make for a fine feature for Drupal 4.6...

wouldn't hurt to have this feature in upcoming drupal.
I would use it for sure, my users are from the same place, with dst

This is an incredibly useful feature and I would like to see it incorporated into the next release (current release is 4.6.0).

Works beautifully.. I'm going to work on getting this to work with event.module.

I have just discussed with killes and rorris an enhancement to this patch which I will implement. We have proposed putting the ical timezone db (http://cvs.drupal.org/viewcvs/drupal/contributions/modules/ical/ical.sql...) in core. We will implement a DST field in that db for each zone which coreesponds with the zones in killes' patch here for dtermining whether a zone is currently in dst or not. Currently, the 5 zones in the drupal_is_dst() function in this patch will be the only regions assigned to timezones. If more regions are desired they can be submitted by the community.

To be clear, this must be implemented before additional development on ical/event.module can proceed properly. We would like this to be in core for 4.7, and move one more step with timezone handling in event module to use this as well.

Discuss!

I agree to put more specific region to DST as it's easier for someone to know which city DST he belongs than which DST Zone (Pacific Standard Time...).
I like pretty much the way that windows manage this.
+1

The patch looks good... however, it doesn't take into account the fact that in Australia, DST starts and ends at 2am on Sunday morning. So these lines:

Should read:

There are also a whole lot more countries (and their specific DST rules) on http://webexhibits.org/daylightsaving/g.html than are implemented in this patch. I'd ultimately like to see an admin interface to let the webmaster add additional DST rules for whatever country they live in (or are developing for). These rules could be entered in raw PHP, or using a somewhat friendlier interface.

Jaza.

+1 on this patch. Catches most cases and at least provides a base for others to work on.

Do not take this the wrong what, but when it comes to dst, this is exemely simplisict.

What would be the best method of doing this is to use the Zone Info files from libc. These not only have all the dst splits, but also all the timezones, and would be the most acurate.

Have a look at Sources of Time Zone and Daylight Savings Data whcih has pointers to where all this information is.

Did you know that Australia has 21 different Timezones and DST areas.

If you could enter the same information from tzdata then this would be the altermate dst system

There has sure been a lot of talk about DST and time zones lately. All I ask is that we hold off on committing anything for a bit. I am currently working on a project that will hopefully fix this issue completely, correctly, and once and for all. :-)

I am in the process of writing a pure PHP library that will be released LGPL that uses the real zoneinfo data files to handle any date (not just Unix timestamps) in any time zone. With this, site admins and/or users will be able to select their real time zone (America/New_York vs. our current GMT -500) and DST will inherently be handled without users having to change their settings. Much of the grunt work is already done. I still have to put all of the pieces together and make sure everything is well documented. As soon as I have tested everything to my satisfaction, I will role a Drupal patch that we can hopefully get tested and committed ASAP. Of course I want to see this get into Drupal first! ;-)

I hope to have everything completed in the next couple of weeks, but based on the way things have been going lately, I can't make any promises. It will definitely be done before 4.7, though...whatever it takes!

+1

This sounds great, but I think we will still need a pure php version that will be able to be used on hosted web sites where we will not be able to get you library installed. It doesn't need to be as effient as the php extension, as sites that at that big will be self hosted and be able to install this library.

One question. Does you extension run on windows. If it does this is another reason to have a pure php version.

This is really the method that should be employed.

When I say "library," I just mean a collection of PHP files as opposed to a single file. There is nothing to be installed like an extension. This is pure PHP that will simply be part of the distribution (a handful of .php files placed inside includes/zoneinfo or something like that). As this is pure PHP, it will work anywhere PHP does. This will essentially give us the benefit and functionality of using the TZ variable on Unix without all of the problems associated with that (i.e. not knowing what zones are available and what there names are, not being available on Windows, etc.).

This sounds good. If you would like someone who has done alot of stuff with timezones and dst (mainly in the contestable electricity inductry in australia, which is required for billing), I would be quite happy to take a look.

The only problem that I see is that it sounds a little too messy for core. maybe this could be made into a contributed module that can be installed if people need the correct timezones.

However this is how the dst and timezones need to be done if we want to get them correct. As I said before and I couldn't believe it, Australia has 21 different timezones and dst areas. The timezone database needs to be maintained.

jhriggs, gordon: Please have a look at crunchywelch's dst implementation for event.module (in 4.6 and cvs). We intend to push this into core.

Great! event_timezones.inc definitely needs to be in core, as it would be used for both user and site configuration. But it should include everything from the unix zoneinfo libraries, e.g. timezone abbreviations used by strftime(), so you can display "PDT" or "PST" instead of "America/Los_Angeles".

so, a complete port of the zoneinfo libraries to PHP is probably what is needed. This would be used for site config, user config and events.

one idea, maybe use mysql timezone tables that are available since mysql 4.1.3?
http://www.visionwebhosting.net/mysql-web-hosting/page0403.htm

not sure how it's possible with licensing, but original tzdata is GPL (at least according to Fedora Core 2 tzdata.spec) then mysql data table contents should be too GPL.

good side is that it should work for windows too http://dev.mysql.com/downloads/timezones.html

It would be great to use MySQL 4.1 as the solution but it probably isn't (yet) possible for Drupal to require MySQL 4.1 when many webhosts are still using MySQL 4.0. We'll have to use our own zoneinfo library. Which by the way will need an update if the U.S. changes daylight saving time.

+1 for this feature, it's irritating to change timezones every time DST is set.

Any updates? Love to see this for 4.7, seems this has been simmering for quite sometime now.

This appears not to have been integrated into version 4.7 as indicated above, at least not as of beta 5. Is this still going to make it in?

This patch fails when applied to Drupal 4.7 rc2:

It is my understanding that crunchywelch's attempts to move his DST implementation from the event module into Drupal core have been met with resistance. This leaves us with no DST solution for 4.7. Would it be at all possible to do this with a third-party module rather than a patch?

Yeah, it didn't make it. We haven't been pushing hard enough. Somebody wanted to make a stand-alone module, not sure it will work, though.

What else can be done on this, then? I'm not much of a programmer, but I would like to help out; this is the one last big feature missing from Drupal for me. Should we start a project where we can better organize this effort? Let me know what I can do to help make this happen.

We could create a group for this :/

What else can be done on this, then? This is MUST have feature for 4.8...

I needed this patch for a 4.7.3 site so I modified it and here it is if someone else needs it..

fixing formatting

New patch against Drupal 4.7.4. Exactly the same as the patch in #29, except that it includes the fix for the Australian region that I pointed out in #11. This patch is still very much needed, IMO.

I second that! Drupal not handling daylight savings is a constant headache for me and I am embarrassed each time I have to explain this fact to customers. Let's get the patch committed asap.

A few more fixes for AU and NZ daylight savings in this patch.

I don't think the patch is implemented this way, but I think the user ought to be able to choose CST, or CDT (where C would be different for the timezone), and then it will automatically calculate whether it is DST currently or not. That would add almost twice as many timezones to the list, but it would make sense to me. Or, you could just do CST, and CST (Daylight Savings) just so the users know what's going on.

Or you could just do a checkbox...

Is there a chance this will go into 5.0, or is it too late?

Had fun trying to apply this patch till I read the patch and then saw #31, it's for 4.7 not CVS. Changing version

Hmmm... can this be turned into a feature request for 5.0 (or the next version if this is considered a major feature)?

Not cool having to go back and forth from -400 to -500 times whenever the clock changes.

--D

This is a fundamental feature, and I think that for 5.0 we should try to do the "right thing", that is, use a database that maps locations to time zone information.

In this scheme users should not select which time zone they're are on anymore, but select where they're located.

The time zone database can them be used to map user locations to their correct time zone plus DST settings.

We must not forget that DST support is not only about changing display time on specific date/time. On many countries DST start/end dates change every year. You may have a post made LASTYEAR-MONTH-DAY at 13pm that have been done on DST, but even if THISYEAR-MONTH-DAY is not on DST because DST start date has changed, last year's post should still be shown as been made at 13pm.

This mean that such database should not contains DST start/end dates for every location, but keep historical data about DST changes.

Fortunately, this is exactly how time zone handling is implemented on most modern OS (including Linux, *BSDs and Un*xes). Unfortunately there's no official support on PHP < 5 for changing timezones on-the-fly, though it looks like setting the TZ environment variable does the trick (see http://www.php.net/manual/en/function.putenv.php).

any chance of it making into 6?

I've always used putenv on non-drupal sites.. e.g. putenv('TZ=America/Buenos_Aires'); then putenv('TZ'); when you're done to go back to the server default.

Now that timezones are better supported by PHP 5.1, how about rewriting format_date(), system_date_time_settings() etc. to take advantage of this functionality if it's available?

links to some of the timezone functions..
http://us2.php.net/manual/en/function.date-timezone-set.php
http://us2.php.net/manual/en/function.timezone-offset-get.php
http://us2.php.net/manual/en/function.timezone-identifiers-list.php
http://us2.php.net/manual/en/function.timezone-abbreviations-list.php

At least on Unix, and php 5.1, wouldn't a combination of:
date_default_timezone_set($userTZ || $siteTZ);
date($formatString, $utcTimeStamp);
correctly format the time, including DST? This avoids writing your own timezone tables, etc.

Does this reliably work on Windows?

For earlier PHP, putenv('TZ=SomeValue') may work, instead of date_default_timezone_set(). I found it problematic (the timezone appeared to be cached between calls to date()).

Does this address and solve the timezone issues?

A backward compatible patch would operate as now for numeric 'configurable_timezones'/'date_default_timezone' values, but use the above mechanism otherwise.

At least on linux, putenv('TZ=SomeValue') works fine: as long as the timezone-name is good (see the PHP appendix for supported timezone-names).

I have created a patch that would introduce this feature on systems that support the new timezone handling in php 5. It degrades to the current system for systems still using php 4. This I think is in line with recent discussions about starting to add in features that only work in php 5 so long as there is some decent fallback for php 4.

So this patch:

1) Adds a function drupal_handle_timezones() which will check if the system can use php timezone handling.

2) Adds a 'zonename' column to the users table where we can store the timezone name as well as the offset

3) On the system date/time settings page and user edit page, checks if php timezone handling is enabled. If so, it presents a list of timezone names to choose from instead of offsets.

4) Adds two functions to automatically calculate the site and user offsets from the timezone name and update the system variable and user records. These are used when the site and user timezone info is updated, and by cron.

5) Adds a cron function to update the timezone offsets once a day.

6) For environments where the php timezone functions won't work, the system behaves exactly as it always has, using offsets.

Since it uses the new php timezone handling, there is no need to load in any lists of timezone names, they can be called using a php function, so there is not a lot of new code needed to do it this way.

Forgot to mark it as a patch...

Nice! Just wanted to mention there is also discussion on g.d.o about this: http://groups.drupal.org/node/3407

It doesn't seem too elegant to maintain the old timezone offsets as separate data entities when you're actually using the correct named timezones.

I was hoping there'd be some way to abstract this out and make the offset available via the php functions -- I didn't realize modules would directly use the timezone offset column, as opposed to $user->timezone, calling format_date(), etc.

The only way to literally simulate the offset column would be to use MySQL's timezone support -- but I'm guessing that's not an option for db compatibility ;)

Thanks for the patch, I will try it soon. Will be a huge step forward for radio, TV and other sites where precise times and timezones are important.

My original thought was to get rid of the offset column, but didn't for two reasons:

1) Backwards compatibility. There is no way to continue to provide the existing behavior for sites using php4 without it.

2) It actually is sometimes useful to have it in there. As a part of the database you can use it in SQL queries, like Views. Plus there's not that much overhead for that one small column.

And the database compatibility problem is worse than you may realize since even if you ignore other databases, mysql's handling of timezones varies depending on the version you are using and the way it is configured.

You should make sure that the tz identifiers are translated on output.

Actually, that just occurred to me, too. Not sure of the best way to do that since we're bringing in the list using php. Maybe iterate over the list and apply t() to each, then store it?? I tried to see if php does locale adjustments on it, but I don't think so.

Does Drupal use setlocale()? If so, maybe we can do something using strftime's %z to output the translated name of the timezone.

I see how the timezone offset column needs to stay for now, in case a site is reverted to PHP4 environment.

But the overall idea here should be that the old-style timezone offset is not used by views and other modules, because the offset is incorrect half the time (if it's currently summer, a date/time in winter will be have the incorrect time displayed, and vice versa). Views needs access to the correct timezone-based time display.

strftime (on my system) returns timezone abbreviations like PDT, which don't vary by locale setting. Should be fine to just wrap in t() whenever a timezone string is being displayed.

There is really nothing wrong with using the offset, so long as it is correct. This patch should correct it for systems using php5, and systems using php4 will be in the same position they are in now. You could go through the code and remove all references to the timezone column and find different ways to do it, but it will involve a lot of extra code to provide two alternative different ways to do everything, one that works in php4 and one that works in php5. That seems unnecessary when everything will work just fine as long as the offset is correct.

strftime has two strings for the timezone, %z and %Z. The exact thing they return varies depending on how the system was configured and the library that was used, but generally one returns the full name of the timezone and the other returns the abbreviation. If set_locale() has been used, they will return the translated values, so if you see no difference by locale, it appears set_locale() has not been set. If we're not already using set_locale() in Drupal, we should consider it -- there are a number of php functions that will automatically adjust to local formats/translations if it is set.

Anyway, you do NOT want to combine strftime() and t(), you need one or the other. t() expects the base value to be in English, but stftime is supposed to be returning a value in the local language, and we don't want that wrapped in t(). So we could use t() on the php timezone name, which is in English. If we do that, someone will need to provide translations for all those values. The ideal would be if we can use strftime and have it work correctly (i.e. if set_locale() has been used) since then the translations will be done by php.

Well the crux of this issue is that a generic offset is useless so I would say isn't worth using in a SQL query, it has to be calculated on the fly for each date -- this is what the new PHP5 timezone support provides. If my user account has an offset of -07:00 in summer this will be incorrect if I am actually trying to display a past or future date/time in winter. In other words it's incorrect half the time..

I was suggesting to use t() and not bother with strftime(), which does not (at least on FreeBSD 6.2) provide localized timezone names (it only localizes the day of the week) nor return full timezone names (only an abbreviation).

I misunderstood you on strftime, I thought you were proposing to wrap the strftime result with t().

On the question of whether or not to drop the timezone column, it would be interesting to see exactly how many places in core it is actually used. If there are not many, maybe it is worth rewriting the code and getting rid of the column. Are you interested in digging into the code and figuring out exactly where that column is being used now?

I was also mulling over other options. One other approach would be to rewrite timezone functions a bit so they use wrapper functions (drupal_date(), drupal_time(), etc.) that will use the php5 functions if php5 is installed and include an .inc file that does some sort of workaround timezone handling for php4. Then we could get rid of the file completely once we can require php5. The workaround .inc file could have the timezone list and maybe do something using putenv(TZ) to get quasi timezone handling in php4. I tried using putenv() in early versions of the date module and it didn't work in all situations and actually created errors in some, so I need to dig back through my issues to try to figure out what the problems actually were.

I got curious and did some digging around and that timezone column is only used in a couple places in core, so maybe getting rid of it is easier than I thought. I have also been playing around with the idea of using wrapper functions for each of the new timezone functions and a php4 .inc file to make php4 mimic things that php5 does natively and I'm starting to think we could make that work and it might be a good solution.

Why don't we take this discussion back to http://groups.drupal.org/node/3407 and flesh out an approach that we can bring back here as a patch?

@killes, you've done lots of work on this already and I suspect we can use some of what you've already done in the php4 .inc files, so what do you think?

I finally installed drupal head so I can try out this patch. But, it no longer applies :(
Too many changes in system.module and system.install. Can you re-roll?
Also, it's preferable to make a patch with unix line endings (rather than DOS).

Btw, the main thing I'd like to add to this patch is, for format_date() in common.inc to use the zonename to do a correct offset calculation each time it's called. Of course if PHP timezone support is not present it would use the offset.

I think it would be a good idea to base the time Drupal uses on the timezone the server is in. Users can then override this setting for themselves by choosing their own DST/summertime options. Because lots (if not most) of servers are in the country they serve this would be a good start. Advanced DST controls could be made later if possible/necessary.

subscribing

we could call date_default_timezone_get() (if the function exists) during drupal installation and use it to configure the site timezone. This function takes into account the TZ env variable (server timezone) in "guessing" the timezone. this actually isn't that useful for me personally since, all my servers are all in one timezone but most of the sites I install are for other timezones.

How does http://drupal.org/node/105039 affect this patch?

Nice work, KarenS. I think you've got things going in the right direction, but there are a couple things that concern me about the patch. Please correct me if I'm wrong in my understanding of how it works.

This patch appears to not really implement a timezone, but merely apply a different offset based on timezone that gets updated everyday. For example, I live in Oklahoma and am currently in CDT, so my timezone would America/Chicago. Now, let's say I posted a comment back in January at 1:00 PM CST, the offset is -0600, but now that it's summer, it would say the comment was posted at 2:00 PM because my offset is currently -0500.

Based on that simple scenario, and the fact that daylight savings time doesn't always happen the same time of the year, it seems like we should store the timezone, and then give that information to whatever function formats the date.

Anyway, I've got more thoughts, but I'm putting the rest of my discussion at http://groups.drupal.org/node/3407

One thing I've been thinking about is that, having to chose "America/Chicago" or whatever from a dropdown may be too user un-friendly, for those who don't know their unix timezone. We may at least want a contrib module with the world map that operating systems use. of course, that may be pretty unwieldy to reproduce in a webpage ;)

I just posted some more observations/comments in the gdo link, I'm trying to see if we can get a small patch into 6x before code freeze that will at least start moving us in this direction.

As to the timezone names -- there are actually multiple names for every zone, so Wisconsin could be 'America/Chicago' or 'US/Central'. I think most users would have no trouble understanding 'US/Central', for instance. The downside is that the 'US/...' zonenames are at the very bottom of the list, so you might not realize they're there.

Could the timezone be specified with an ajax autocomplete callback? Then we can stack in as many variants as we want and users can just type "US/C" and start getting zones.

Indeed, a small patch for D6 would be great to get timezones moving properly (pun intended :).

There's lots of discussion about this going on at http://groups.drupal.org/node/3407. At this time I'm leaning toward creating a patch to add a second column to the user table so we can track both a real timezone and the current offset. Some other things, like incorporating that into format_date() may be too complicated to get done before freeze. But at least creating a method to store accurate timezone data and setting up some basic timezone functionality would make it possible for contrib modules to take it further until we get another chance to make changes to core.

I'm working on a patch. Hope to get it posted soon.

KarenS: I think adding a second column to the users table is the way to go, I've chosen this appraoch for my event 5.2 rewrite.

In working on this, I thought it would be best to break it down into several patches. I'm attaching the first of them. You can see a discussion of the reasons behind this at http://groups.drupal.org/node/3407

The patches be:

#1) Patch core to rename user->timezone to user->offset and create a new user->timezone that will hold the timezone name; alter system and user timezone forms to accept a timezone name instead of an offset in systems using php 5.2+; add some hooks so other modules can jump in and do things with timezone info, but otherwise make no changes to timezone handling in core. In other words, mostly just create the necessary infrastructure to do more with timezones.

#2) Patch common.inc to restructure format_date so instead of format_date($timestamp, $type = 'medium', $format = '', $timezone = NULL, $langcode = NULL), it becomes format_date($timestamp, $type = 'medium', $params = array()), where an array of params can include things like 'timezone', 'langcode', 'offset', 'format', etc. Most core calls to format_date() set only the timestamp and type, so this would make it easier to add in new arguments to format_date without making very many changes in core.

#3) Once format_date has been restructured, patch it to properly handle either offsets or timezones.

Even if we don't get past the first patch, we'll start to get enough into core that we can do more with contrib modules.

In my patch I am renaming functions so they use the 'date' space, both to make it easier to find and use the date-related functions and as a precursor to moving them into their own include file, as in my proposed patch at http://drupal.org/node/154477.

So we have:

variable_get('date_default_offset'); -- the offset now stored in date_default_timezone
variable_get('date_default_timezone') -- the timezone name;
date_zone_names(); -- the timezone name array returned by DateTimeZone::listIdentifiers()
date_zone_offsets(); -- the array of offsets we now use
date_handle_timezones(); -- true or false, test whether php5 timezone handling is available on this system

we also will have user->offset for the offset value and user->timezone for the timezone name

Note that user->timezone will be empty on systems earlier than php5.2. user->offset may or may not have a correct value in it if user->timezone is being used. I'm updating it when user->timezone gets updated, but not trying to keep it current.

There is also a hook_timezone_alter() to allow you to alter the timezone info when the site timezone is changed, when the user timezone is changed, and to intervene when the list of available timezones is being constructed.

mfb has posted a patch to handle the last two things on the list -- rework format_date() so we can add more parameters to it and add in new php5 timezone processes to the way it handles dates.

Forgot the link to mfb's patch -- http://drupal.org/node/155442.

Parts #2 and #3 of this task are patched here: http://drupal.org/node/155442
Testing is needed! I will hopefully have some time for testing soon myself.

The functions that I used in my patch say they work with PHP 5 >= 5.1.0

So, shouldn't PHP 5.1.0 work? (aside from whatever bugs and vulnerabilities that version must have ;)

The classes for DateTime and DateTimeZone do exists in PHP 5.1, but it seems like those are not compiled in by default. It might be a good idea to do something like:

return version_compare(PHP_VERSION, '5.1', '>=') && class_exists('DateTime') && class_exists('DateTimeZone');

Some were broken in 5.1 and most everything I've read says don't count on it working correctly until 5.2. Plus there were several functions added between 5.1 and 5.2, can't remember which ones right now, though.

Line 251 of this patch should be

+ $zones[$zone] = format_date($timestamp, 'custom', array('format' => variable_get('date_format_long', 'l, F j, Y - H:i') .' O', 'offset' => $zone, 'timezone' => 'UTC'));

well ignore the last part of what I said, shouldn't be UTC ...

Ok I'm kindof in shock, proper timezone support in drupal core!

proper timezone support in drupal core!

...only if we can get lots of attention on this issue so it gets pushed through. There are *LOTS* of open issues and only a day or two to go until freeze.

BTW, I can't make the change you suggest to my patch because my patch will have to go in before your patch and the new arguments aren't there yet until your patch. Otherwise we have to combine them into a single patch.

Anyway, I think this is a good approach to the problem, and I've spent LOTS and LOTS of time trying to find a nice clean way to get proper timezone support in core, so I feel pretty confident it's a good method. I hope others will dive in and help review and push it forward.

I stand ready to make whatever adjustments are needed to get it in. I'll clean it up, cut it up, stand on my head, whatever :-)

Could we try merging the two patches? Whenever I try patching on my machine, whichever I apply second doesn't completely succeed.

OK here's a merged patch. Test away!

This one has 1 character difference, I didn't fully fix a bug that KarenS found.

OK... Just some preliminary testing here. I did a clean install of drupal, applied the patch, and then ran the upgrade script. I don't know if this is a problem with this patch or something in form.inc, but I got the following error:

notice: Undefined index: id in /var/www/thisoneplace.com/drupal-cvs/drupal/includes/form.inc on line 1920.

I saw this warning a couple days ago when working on another patch, so I'd say it's unrelated.

Yeah, I just saw it someplace else, too. Whenever an upgrade is done, no default timezone is selected. I guess this is fine, but should it default to something like "UTC"?

Other than having defaults set on an upgrade, this is working great for me with PHP 5.2.3. I noticed no problems with a clean install, and dates/times are displaying properly. Great work!

My vote would be defaults of 0 for date_default_offset and 'UTC' for date_default_timezone, just to make things identical to the old behavior.

I debated the default and took it out to force you to pick a timezone. Perhaps that should be changed, though. It could either by 'UTC' or 'GMT', the difference is where on the list it puts you. 'GMT' puts you in the middle of the list, 'UTC' at the bottom.

Also, I just realized that we need to translate the timezone list. That was discussed earlier and then I forgot and left it out in my patch. We could also use the new $langcode variable, like:

On my system, which doesn't have a local timezone set, PHP defaults to UTC. So that seems like the "real" default..

Yes, format_date() allows for translating timezone names (and abbreviations) if you put them in your custom date format, so the select form should too.

With a fresh install, if I leave the timezone field select box not set to anything in the installer then php/apache core dumps. As well, if I select the empty timezone option in /admin/settings/date-time, then I get in screen full of notices.

The timezone select box fields should probably be made a required form field.

If empty timezones are generating errors we definitely need to put a default in there and get rid of the blank option.

I tried to apply this patch and couldn't get it to work.

We need to:

1) Insert defaults instead of NULL for all the variables and values.
2) Get rid of the blank option in the array of zone names (which will make it a required field)
3) Add an optional $langcode variable to the zone names function
4) Add translations to the zone names array (probably by doing a foreach over the array to do t($value, array(), $langcode)).

I'll keep trying to get the patch to work or apply it manually, unless someone else gets these things done first...

One more thing, we should also set timezone to UTC in format_date() if someone mistakenly passes in an empty or null timezone.

I didn't have a problem applying the patch on a fresh checkout.

This version should resolve the above issues. Phew that is a lot of strings that need to be loaded from the database..

I just sent an email to Gábor to see if he has any suggestions on how to handle translation of that array. It would be nice to cache it by $langcode or something so we don't keep regenerating that process over and over.

Some comments while I am here :)

1. Why did you move $format to the parameters array? It seems to be used a lot in core.
2. There is also a user timezone setting switch (with jS), which could be complementary to provide some better functionality even if PHP 5.2 is not used. http://drupal.org/node/149092
3. Having date_* functions in system module does not work. That module should only have system_* functions. Either move generic date functions to a date.inc, or if you think they are not that generic, rename them to system_*_something.
4. userzone => user_timezone, sitezone => site_timezone (or even swap the words, timezone_site(), timezone_user())
5. Why is there a timezone_alter? I understand that when time is set *back*, that could be a problem, so you see content submitted in the future, you get notifications about it and so on. Is this supposed to be used for that case? How? Something else to use that for? Why should this alter be used when timezones are listed??? Form_alter() should be perfectly fine for that case!

Seems like the performance problems you mention could be in date_zone_names(). It would be nice to see a time zone listing which is handled in that function. From the php.net docs, it seems that it would be a better idea to split by '/' and translate the two parts separately, so there would not be a *huge* list of strings to translate in the database (this could be a real performance hog even if you don't list the timezones on a page, as all the timezone names would end up in the locale cache (being short strings) on all page views).

1. Why did you move $format to the parameters array? It seems to be used a lot in core.

Because the list of possible arguments is getting so long and many of them are not always needed or not in a logical order. There is now a $timezone argument which is really an $offset argument and we also need an argument for the timezone name. Seems easier to use if you provide an array like 'timezone' => mytimezonename, 'offset' => myoffset. This will make it much easier to add other optional arguments in the future, if needed.

2. There is also a user timezone setting switch (with jS), which could be complementary to provide some better functionality even if PHP 5.2 is not used. http://drupal.org/node/149092

Yes, that would be nice but I was trying not to make this patch any larger than it already is. Also, that doesn't seem to work for automatically setting a timezone name, only an offset, which will be of limited use for the php5 stuff.

3. Having date_* functions in system module does not work. That module should only have system_* functions. Either move generic date functions to a date.inc, or if you think they are not that generic, rename them to system_*_something.

I want them in a separate file since there are potentially more date functions coming (see my patch at http://drupal.org/node/154477) I didn't think of doing that already in this patch, but would be easy enough to add.

4. userzone => user_timezone, sitezone => site_timezone (or even swap the words, timezone_site(), timezone_user())

You mean rename the functions?? Not quite sure what you're saying.

5. Why is there a timezone_alter? I understand that when time is set *back*, that could be a problem, so you see content submitted in the future, you get notifications about it and so on. Is this supposed to be used for that case? How? Something else to use that for? Why should this alter be used when timezones are listed??? Form_alter() should be perfectly fine for that case!

timezone_alter is to provide a hook so a contrib module could provide proper timezone support in systems that won't support native timezone handling (anything less than php5.2). It would be way too cumbersome to do that in core, but I wanted to provide ways for a contrib module to jump in and alter the timezone list itself, not just the form.

Seems like the performance problems you mention could be in date_zone_names(). It would be nice to see a time zone listing which is handled in that function. From the php.net docs, it seems that it would be a better idea to split by '/' and translate the two parts separately, so there would not be a *huge* list of strings to translate in the database (this could be a real performance hog even if you don't list the timezones on a page, as all the timezone names would end up in the locale cache (being short strings) on all page views).

Yes, the date_zone_names is where the problems are. I'm not familar enough with the way the translation system works to know exactly where and how we could do things like cache some of this stuff, so we're looking for ideas on how to provide translated timezone lists.

Splitting the timezones and translating each part separately wouldn't help, you still have just as many unique city names.

It wouldn't be hard to store the whole translated array in the cache table. I'm just not sure how to deal with the details of expiration, etc.

Another though on bringing the zone names list under control, there are many things on the list that are duplicate ways of creating the same zone. Instead of using the php5 date functions to create the list, we could create our own, shortened list, then translate that. This would also make the list work in php4 (although the native date calculations still won't work there). This could be feasible because that list doesn't really ever change.

For instance, I think all the 'America/...' names are duplicated by country and could be removed (America/New York is the same as US/Eastern). We could also remove the generic zones (+500, etc). Doing that we could maybe cut the list in half (which would also make more user-friendly).

It will still be a long list and we need a way to cache it, if possible.

Karen,

1. It could still be, that we keep often used optional arguments as simple arguments to simplify our lifes.
2. I did not mean folding that into this patch, just pointing out that others are working on making the offset handling better.
3. No way to add date_* functions into system.module
4. Yes, rename, make them obvious.
5. You never even call the function if there is no timezone support (but only offset support), so the alter would have no function there. I don't see the reason to have it in then.

mfb is right, that the city names will still be unique but splitting the time zones for translation could help a great deal for translators to not translate the same string prefixes all times. Like Africa in all the Africa/* timezones. The obvious problem with these short strings is that the locale cache loads them on *all* page loads, so this becomes a serious performance hit on every site using locale and proper timezones.

the US/* Canada/* Etc/GMT* and other oddities are old timezones, that are only in the timezone database for backwards compatibility reasons.

These are the currently valid timezones: http://en.wikipedia.org/wiki/List_of_tz_zones_by_name

The America/* timezones are the correct ones to use for the US.

Gábor, I just wasn't sure if translators would want to be locked in to the "Continent/City" format, they might want to translate "America/New_York" to "US Eastern".

What about this as a proposal to be able to get this patch in before code freeze:

1) Remove the 'date' functions to their own date.inc file (which is going to become more and more necessary)
2) Remove the timezone_alter() hooks for now (I'm not sure there will be any way to provide a contrib module with timezone support that way, but that may just have to be the way it is)
3) Rename the functions you wanted renamed.
4) Roll back the changes to the way format_date() is structured, but leave the new date handling capability. It will just pick up an additional $offset parameter after $langcode (not my preference to do it that way, but it's not worth holding this patch up).

That still leaves the timezone array to be figured out. If we're going to remove the hooks in the function, I'd prefer to hard-code the array so it will be available on any system. Plus then we can remove the deprecated zones to make the array a bit smaller. Maybe we should even store it in the database so it's not held in memory all the time.

Still not sure what to do about translating it. Even if we cache the translated array, we'll still have all those translatable strings in memory.

Here is a patch with those changes.

KarenS: format_date() is used with the offset in form.inc map_month() function, that instance will need to be tweaked.

To avoid code-bloat we could also use PHP's built-in list, but display only those zones beginning with Africa America Antarctica Arctic Asia Atlantic Australia Europe Indian Pacific, and UTC. Thus drupal only needs a much smaller array of continents and oceans.