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.

+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.

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:

    case 4: // Australia
      // start of DST  (last Sunday in October)
      $dststart = strtotime("-1 week sunday GMT", strtotime("1 november $year GMT")) + $timezone;
      // end of DST (last Sunday in March)
      $dstend = strtotime("-1 week sunday GMT", strtotime("1 april $year GMT")) + $timezone;
      break;

Should read:

    case 4: // Australia
      // start of DST  (last Sunday in October at 2am)
      $dststart = strtotime("-1 week sunday GMT", strtotime("1 november $year GMT")) + 7200 + $timezone;
      // end of DST (last Sunday in March at 2am)
      $dstend = strtotime("-1 week sunday GMT", strtotime("1 april $year GMT")) + 7200 + $timezone;
      break;

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.

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:

$ patch -p0 <dst_0.patch
patching file includes/common.inc
Hunk #1 succeeded at 844 (offset -116 lines).
Hunk #2 FAILED at 1630.
1 out of 2 hunks FAILED -- saving rejects to file includes/common.inc.rej
patching file modules/node.module
Hunk #1 FAILED at 1053.
1 out of 1 hunk FAILED -- saving rejects to file modules/node.module.rej
patching file modules/system.module
Hunk #1 FAILED at 216.
Hunk #2 FAILED at 249.
2 out of 2 hunks FAILED -- saving rejects to file modules/system.module.rej

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?

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..

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).

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.

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.

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.

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.

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:

function date_zone_names($langcode = NULL) {
...
}

which, if I understand it right, would allow you to ask for a timezone list for a specific language.

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.

This is the same file as post 104, except I took out the carriage returns.

I'm assuming this is not getting in to 6.x, so I'm in the process of trying to add this into the Date API as a way it can be used in both 5.x and 6.x.

@Gábor - I've been thinking about the problem of translating the timezone names (and this would apply to other long lists of infrequently used values like country, county, or city names, for instance), and I thought it might be good to start an issue to discuss how best to handle that in the next version. I opened an issue at http://drupal.org/node/155954 for this purpose.

Bumping to 7.x.

Well, it looks like we can count on Drupal 7.x requiring PHP 5.2. I guess that means we don't have to worry about supporting offsets. Although some may want to for old contrib modules, I think it would be a better idea to just switch to true timezones only.

[Drupal++]

Subscribe

This is what I was working on during the code sprint at MIT. The patch we were working on for Drupal 6 can be vastly reduced -- removing all the PHP 4 support. But on the other hand, there is a lot more we can do: For example, add a full date class to Drupal 7, and some schema changes, like moving all integer timestamp fields to GMT native datetime fields (which allows us to store a much wider range of dates, and given that contrib modules use datetime fields, it makes sense to not have a mix of integer and datetime fields).

What I did so far was prepare some benchmarking scripts to compare integer and datetime fields, and I'm going to commit this to my sandbox soon. If anyone else is interested in this, I could use some queries or use cases to benchmark -- please send 'em my way.

P.S. regarding a date class, I was wondering if there are any agreed-upon practices for adding core classes? e.g. should they all be in a "Drupal" namespace.

So far there is no standard for class naming other than following PHP standard: ClassName::methodName().

I was doing a little toying with dates recently, and have a DateValue value object class that I can throw your way if you're interested. It's a bit nicer to work with than the built-in DateTime class. KarenS should have a copy as well. I'm in general +1 on using an actual DateTime or wrapper thereof for all of our date/time handling.

Sure, I'd be interested in checking it out.
Off-topic from timezones, but still date-related: I did some benchmarking on int vs. datetime fields, running queries on identical generated data. I tested "SELECT...WHERE..." and "SELECT...ORDER BY..." type queries, both using and not using indexes. On mysql 5.0.45, I found that queries on native datetime fields took 1% longer than queries on integer fields. On a untweaked postgresql 8.2 install I found a larger penalty. Note this was just testing raw db performance -- running thousands of non-real-world queries on tables containing nothing but dates, since I didn't yet create an actual drupal patch to benchmark. But still it does seem likely there would be at least some small performance penalty in switching from int to datetime.

I worked with date and time fields outside of drupal in past and ported MySQL 4.x code to MsSQL2K. This was a very bad experience... an integer is for sure much much easier and should be really faster.

mfb: Have a look in my sandbox: http://cvs.drupal.org/viewvc.py/drupal/contributions/sandbox/crell/datev...

KarenS has a patch in the works which adds a date api to drupal. specifically, site and user timezones will be specified by name, not by offset. then the php5.2 date handling emits the date for the user considering his timezone and DST and so on. coming very soon.

There's a huge amount of work that has been done in the 5.2 version of the Date API that I was hoping to present as a core patch very soon. Some of the things included in the Date API are:

1) PHP 5.2 date handling is used, which properly handles timezone conversions, including dst.

2) Site and user timezone settings store timezone names instead of offsets for use in date manipulations.

3) Built in jQuery date and time pickers are included.

4) Automatic, self-validating date form elements can be created by adding '#type' => 'date_select' or '#type' => 'date_popup' and a couple other parameters to forms.

5) Cross-database date SQL for comparing dates and extracting date parts out of timestamps, ISO dates or datetime fields -- created by analyzing MYSQL and POSTGRESQL datetime documentation to return the same results in both. This has been in use in the Date module for over a year now, so has undergone some testing.

6) A fairly complex iCal parser to convert ical files into organized arrays of event information to make it easier to both import and export date data using iCal rules.

7) Functionality to create repeating dates -- both a form element that will allow a user to select repeat rules and a function to take a rule and return an array of the dates that fit that rule. This part might be too much for core and could remain a contrib module.

8) And it all comes with a number of simpletests that test the API, the timezone conversions, the repeating date logic, etc.

In addition to the API, I hoped to convert all core date fields to native datetime fields to make date manipulation work better and get away with the awful cludges you have to do to convert timestamps to something the database can understand as a date. I'd be interested in benchmarking the performance of datetime fields vs integer timestamps, but it seems likely that dates in the native datetime format should perform fairly well and would be *far* easier to manipulate in a database-agnostic way if you need to do things like extract the month from a date (and they also have the advantage of being human-readable).

I proposed a session at DrupalCon to discuss this and it got bumped down to a BOF, which mostly got overlooked, but I've been trying hard to get some discussion going about this topic for the last year or so, both here and on g.d.o.

I did not create this as a date class, but as procedural functions, and I'm not sure if there is any advantage in dumping all that work (and all the testing of that code that has already been done in the last six months or so) and starting over to create this as a class.

Re: switch to datetime columns, any ideas on how much of a performance penalty is acceptable? So far in my raw tests of queries on 200,000 rows of dates, queries on datetime dates took 1% longer than queries on int dates. But hopefully there would be smaller impact in the context of a drupal site. I'd like to go ahead and try patching drupal core to use datetime and running benchmarks on things like /node, /tracker, etc. (if there's a decent chance this won't be wasted effort ;)

One thing to consider in the evaluation of performance is what happens on a query like "SELECT MONTH(FROM_UNIXTIME(created))", where created is an integer that has to be cast as a datetime field, vs "SELECT MONTH(created)" where created is a datetime field that doesn't need to be converted. I'm assuming the first will be slower because of the FROM_UNIXTIME conversion. So it's not just simple selects that need to be analyzed, but also selects that are doing date manipulation.

In other words, maybe the cost of FROM_UNIXTIME offsets whatever performance cost there is in the simple selects of datetime fields over integers.

If 1% is the only performance cost of using native timestamps, what's the feature benefit? 1% is, I think, easy enough to swallow if there is a compelling reason for it (in terms of capability, cleanliness, stability, etc.) There are other places in core where we should be able to get a performance boost this time around to offset small losses like that, if they're with cause.

I didn't see a significant difference between SELECT MONTH(FROM_UNIXTIME(created)) and SELECT MONTH(created) when running these queries 180,000 times. But I also don't think this is a fair benchmark since they're not returning the same results. MONTH(FROM_UNIXTIME(created)) is giving me the month in the database's timezone. MONTH(created) is giving me the month in GMT. In real-life you want neither, you want the month in the user's timezone. This could be done in mysql with CONVERT_TZ() or on the php side.

1% over 200,000 queries isn't bad at all if you can store pre-1970 dates...

Date module doesn't already have a javascript timezone picker does it? because I figured out a way to make drupal core's timezone picker work with php timezone names. I can get the timezone abbreviation and offset from javascript and pass it in a ajax call to timezone_name_from_abbr(). patch coming soon (cross-browser testing..)

Although there's still lots to discuss/work on related to dates, I wanted to upload a patch to get timezones working on Drupal 7. This is just the bare minimum without a new class or datetime schema. I might even suggest to commit this patch and then do more work on another issue.

I tried to get javascript timezone auto-detection working as well as possible. On Windows clients (FF and IE6) it does not seem to detect all timezones, but it works at least most of the time there. Timezone detection should work better on Linux FF (which passes in via ajax a timezone abbreviation that is missing from the Windows platform).

Don't think it matters but one of the js variables wasn't declared in the first draft.

Re: javascript detection of the timezone name -- I tried lots of things to get that working and dropped it because browsers are so inconsistent about how they report the timezone that I couldn't get it working. If you can get it working reliably that would be great :)

Re: CONVERT_TZ, it sadly turns out we cannot use it because many hosts do not have the timezone tables available by default. I tried using that in the 5.2 version of the Date module and immediately starting getting numerous reports of people who were on hosts that did not have the timezone info available in the database. Plus I found that there's no comparable function in POSTGRESQL -- well there is a function but it doesn't behave the same way at all. So at the moment I don't think we can do timezone conversion in the database that way and will have to do it by adding and subtracting offsets instead. See http://drupal.org/node/218479 and http://drupal.org/node/220663 for more details of the problems we ran into.

Re: your patch -- one thing you are doing is setting the default value for users to UTC which is a mistake I made early on in the Date module. That makes the timezone wildly wrong if they edit their user settings without actually selecting a timezone. What I did instead was offer an empty value and set that as the default. Then if the timezone name hasn't been set it will be empty, and you'll fall back to the site timezone name instead, a much better solution. You are also using the current user timezone field and I would instead use a new timezone_name field (for both user and site timezones). Anyone using the Date module will already have that set correctly and we can start it out with a NULL value, as noted above, to know when the user has actually set it.

Thanks for feedback! I'll roll another version. Re: timezone_name, Yes it should be supported, I'd forgotten it adds a column to user table. I do like to avoid long column names.. for aesthetic reasons I'd just copy data from it but maybe i'm just being nitpicky.

I'd like to say js timezone detection in the patch is "good enough" but it could use testing. It will be impossible to have it working perfectly without hitting limitations of browser/platform inconsistencies and of timezone_name_from_abbr() itself, well unless we want to test every scenario and write a crapload of javascript.

After reading through all the related issues, it has become apparent that there is no ideal solution short of a full Olsen implementation in core; which would put us out another 4 years. :-(

Here's the option list:

1. - Database support of timezones. As many have noted, it isn't possible to get full support by ISP's, as well as this option is unique to the database backend used.

2. - PHP 5.1.0+ timezone functions. These require a PECL module for updates - which may have ISP support issues, but appears to be supported in PHP core and cross platform. An additional issue is the PHP version requirement.

3. - Native Unix/Linux Olsen databases (/usr/share/zoneinfo). Not useful on Windows; which has its own version of Olsen-like tables, but would require different access mechanisms. For instance the easiest way to set the timezone in Linux is with putenv(TZ=....), which has issues in Windows implementations.

4. - JQuery the client. This has no major server drawbacks, but significant client issues.

Of all of these, number 2 seems that it would have the least amount of headaches; but still may lack universality. For this reason, it would make more sense to implement timezone corrections via plugin. Due the the many modules this would touch, it would require core support. The fallback can always be the offset we have now - but which is incorrect for half the year without manual intervention by every member who has a zone set (and lives in a DST region; a majority of the internet population).

It also seems ridiculous to store date/time as anything but date/time strings (YYYY-MM-DD HH:MM:SS) in UTC. Unix timestamps simply don't have the versatility and range. I had no idea timestamps were (still?) being used in Drupal. As others have pointed out in the related threads, there are a lot of places where performance can be adjusted to offset the sacrifice of having human readable time attached to our data.

The process (as many of you know) is that assuming all times are stored in UTC, there are a few simple functions which are indispensable. Call them whatever you wish, but I'll use these names for description:

enumerate_zones() to show what possibilities are available.
tz_set() to change the session to the user's zone, or the server zone if this cannot be determined.
to_utc() converts from localtime for any data storage (or date-based queries!)
from_utc() converts anything from the database for representation.

Providing math to calculate the offset of two arbitrary zones is largely irrelevant in an application like this, except for the two cases of UTC conversion. There's simply no general need to convert between Australia/Perth and America/Los_Angeles. Yet this is where most designers start.

Now in the Event module, it's not quite so cut and dry. That's outside the scope of this dicussion but suffice to say many people would be alarmed to have Christmas start at 3AM on Dec. 26th. Some events need a special case to denote that they are absolute no matter what timezone applies. A world-wide podcast would not fall in this special case. It needs to be adjusted for localtime or the visitors would tune in at the wrong time.

Anyway, my proposal for a long-term solution is for stub functions for these key functions, with module callbacks so that they can be over-ridden, and with default implementations which use the simple non-adjusted fixed offsets we have now. Then perhaps a few of us can come up with a good plugin or two comprising the most robust solutions available. This takes the pressure off of core to have a solution for everybody.

A solution for everybody is likely to be a bloody mess. But it might not take much more than a few hours to make a robust plugin. I've done the Linux /usr/share/zoneinfo for other community systems and it works well. This would work for anybody on Linux, regardless of PHP version. The PHP timezone functions will work for anybody with newer systems. Folks running on Windows/PHP4 servers would be stuck changing their offset twice a year; but I don't know any other way around that one.

I don't see a bright future in pursuing the MySQL function route, although it's arguably better than the fixed offsets we've got. The JQuery client check likewise is a very clever solution and better than what we've got now. At least if it goes wrong you can blame the client.

Personally I'll take any solution that works. I have Linux Olsen tables, I've got zones in MySQL and I've got PHP5.x. My client time is OK for JQuery, but I can't speak for my 'customers'. But none of these options seem like they're suitable for core, so let's not put them there. Let's just get the stubs/module-callbacks in so that folks who are able to use them can. All we need from core are the stubs, and a check to make sure that the appropriate functions are called in all the proper places.

Apologies for the long post.

macgirvin, could you try the latest version of this patch and let us know what you think? We already know that Drupal 7 will require PHP 5.2 (and MySQL 5.0, although not necessarily with timezone support loaded into the MySQL server). Re: the patch, note I do need to reroll it with a few fixes and upload here (soon as I find a moment).

@mfb: You cannot use MySQL functions for this daylight part. There could be MsSQL, Oracle, Postgre and others... and PDO support came in... maybe this could make things easier in a way i don't know http://drupal.org/node/225450.

Although I was using this issue to do some date-related database benchmarking, it's not related to the patch (please take a look and test it), where I'm just using PHP timezone functions. Anything related to database probably belongs in some other issue (whether core or date module) since this one is getting pretty long ;)

Patch installed (to be fair I manually back ported it to 6.1, because my customers aren't going to wait for 7.x). So far it seems to work as advertised as far as setting zones. I've still got to run it through the paces to see if we actually get the correct time, but it's straightforward. The folks here in Australia spit the dummy (get upset) when you mess up or ignore their timezone, so I'll find out soon enough if it has any problems. We're going through a daylight time change in another week or two and I'll be able to really give it a whirl.

PHP 5.2 includes a full Olsen implementation. That's why we wanted to make that the minimum version for Drupal 7. :-)

PDO won't help in the timezone case. The main stumbling block is that MySQL has an optional look-aside table for doing timezone translation, but most hosts don't have it installed so we can't make use of it. PDO is just the interface to the database.

I've been able to verify this patch using the 2007 timezone data for Australia, which resulted in daylight savings changing this morning.

Note: In fact there were several changes to the daylight savings rules here in Australia this year and many service providers have not yet upgraded. So this is actually checking the old tables which cutover today, using my U.S. hosting provider. This timezone data is incorrect for 2008, however it proves the patch works fine.

I'll have another important data point next Sunday (Saturday back in the states) which is the cutover day under the new Aussie rules, and which will roll over on my Australian test system that has all the current timezone patches (installed in linux, php, and mysql).

Re: d7.timezone2.patch
As the default value, date_default_timezone_get() may be a better guess than UTC, at least for people who administer their own server or who use a local ISP (in their local timezone).

@mfb:

Do you have any more recent rolls on the patch? I notice one was promised a week or two ago.

I've spent the last week or two re-acquainting myself with all the Drupal date/time issues (I had abandoned Drupal a couple of years ago over these and similar bugs). The date/time code is still a mess, as it was back in late '05. It prevents me from recommending Drupal to many of my Australian customers who are quite sensitive about the ability to track this country's frequent DST policy changes. Using some form of updateable timezone tables is a huge step forward. We need to push this harder to get into 7.x though or it will slip out yet another two+ years (Drupal major release + a year for modules to catch up).

FYI: I noticed patching this into 6.x that I was still getting an errant call from somewhere (which I haven't yet tracked, but is on a contrib-free install) using numeric offsets which I hacked a quick fix for thusly:

-  if (!isset($timezones[$timezone])) {
+  if (!isset($timezones[$timezone]) && (! intval($timezone))) {
    $timezones[$timezone] = timezone_open($timezone);
  } 

It isn't logically correct, and this isn't the target patch version; but pointed the potential need for some form of exception handling when hit with a numeric offset.

I'm also aghast at the potential performance issues related to the current format_date() function, which can get called a lot of times over the course of a page load. This needs a new issue, to avoid recursing on itself over and over (and ultimately falling into somewhat costly libc functions) for each character of the format string. Granted this is done to allow translation of the string elements returned by date, but is horribly costly. My thoughts are to just call date/gmdate once, with an altered format that specially prefixes the elements which need translating, then translate all the specially prefixed elements on the final result. Thoughts appreciated, but should be directed at a new issue (which hasn't yet been created as I'm writing this).

I managed to work out something which should be an improvement over the current way that format_date works for the string translations, as I mentioned in #142. I'm going to post the rough code here so that it doesn't get lost on my hard drive. If mfb would like to work this routine into the patch it would be a huge improvement. This is just a proof of concept at this point that will need a bit of work to integrate and review. It does away with a lot of the messiness of format_date. It could be even better if we didn't have the long-month-name to deal with, but we've got the hand we were dealt.

  // Replace entries in $format string which need to be translated with escaped string '+=X=+'
  // where X is a format specifier which requires translation. The 'F' format specifier requires
  // special handling because in English (for example) May is both a short and long month name.
  // We need to specify for the 'F' format that it is the long month name we will be translating.
  // These patterns specifically exclude any pattern specifier which has been prefixed with '\\'
  // (octal 134).

  $format = preg_replace('@(?<!\134)([AaeDlMt])@','+=$1=+',$format);
  $format = preg_replace('@(?<!\134)(F)@','+=!\l\o\n\g-\m\o\n\t\h-\n\a\m\e F=+',$format);

  // Make a single call to gmdate using our specially formatted string.

  $date = gmdate($format,$timestamp);

  // Now look for anything that we escaped, and translate it.

  if(preg_match_all('@\+=([-! a-zA-Z]*)=\+@',$date,$matches)) {
    $arr = $matches[1];
    foreach($arr as $a) {
      if(substr($a,0,16) == '!long-month-name')
        $date = str_replace("+=$a=+",t($a,array('!long-month-name' => ''),$langcode),$date);
      else
        $date = str_replace("+=$a=+",t($a,array(),$langcode),$date);
   }
  }

  // $date now contains the correct translation of the date format.

UPDATE: 06-APR-2008 This issue has now been split off into #243129: Refactor format_date() to avoid multiple calls to date_format() - but as it affects some of the same code as the daylight savings patch, you might wish to subscribe to the issue so as to avoid conflicting patches.

First Sunday in April 2008. Sydney, Australia - Daylight Savings Time changes. This event usually strikes fear into the heart of any software developer.

I change all the house clocks, the car clock, the sprinkler and heater clocks. Check all the PCs and phones. Most of my systems and web communities changed the time last weekend (wrong - that was the 2007 rule; but today they become right again).

Now it's time to login to every Drupal site I've ever visited and update the time there, because it is the only web service I'm connected to that isn't yet aware of Daylight Savings. But wait, my system has been patched (patch in #131). My development sites actually display the correct time. Yay! It works! That still leaves me with about 80 or 90 Drupal sites to change, but there's light shining through the darkness. Maybe someday this will no longer be necessary.

Excellent :)
I'm still workin on this patch but have been busy elsewhere lately. The main thing that needed testing in #131 was the javascript timezone detection which I only tested on a few systems (it is sensitive to platform issues on the client side -- different operating systems and browsers). You can test whether or not it chooses an approximately correct timezone both when installing a new site and when creating a new account (it can't be perfect, it won't know which South American city to choose for example).

I rerolled the patch, so it should once again apply to HEAD. I moved the user update function over to user.install, and copy data from timezone_name if the column exists. As someone suggested I used date_default_timezone_get() to guess the timezone during installation. Although, this form has javascript auto-detection so it will mostly come into play for something like an automated script which is installing drupal without javascript.

@KarenS: you wrote "one thing you are doing is setting the default value for users to UTC which is a mistake I made early on in the Date module. That makes the timezone wildly wrong if they edit their user settings without actually selecting a timezone. What I did instead was offer an empty value and set that as the default."

I think I had this correct before, but please clarify if I'm missing something: If user-configurable timezones are enabled I am defaulting users to the site-wide default timezone (both in hidden field at the time of registration, and on their edit page if they don't have a timezone set when editing). This is how it works now and seems (to me) to be the most user-friendly approach for most sites (for example if timezones are suddenly made user-configurable, it seems correct to pre-select the default rather than '' => 'Choose a timezone'). There is also javascript timezone detection during the registration process, so it would only be users who register without javascript who get the site-wide default.

Chasing HEAD.

Chasing HEAD.

subscribing

I've followed this for a while, but it'll be nice to have updates sent directly to my mail... so subscribing. :) (This is also a +1 from me to the patch's purpose.)

Chasing HEAD.

Chasing HEAD.

This would be an ideal patch to write test functions for. The tests could forward the time to see if the time adjust accordingly.

The code looks good, only the Javascript code is a bit puzzling. I'd love to see a bit more documentation around that. It's not quite clear what $dst is for. I think it is to indicate that we should use the offsets but I'm not quite sure.

Also, a lot of knowledge is captures in this issue, but very little of that actually flows to the patch. As this is such a hard problem to solve, I recommend that we add a bit more code comments to the patch.

I also think this warrants a CHANGELOG.txt entry.

OK I started on some tests. Any suggestions on other tests to add?
I also added some comments to the javascript and in a few other places.

I also added a test for user-configurable time zones.

Subscribing...

This version adds a CHANGELOG.txt entry.

Marked #11077: Introduce Daylight Saving Time for Drupal as duplicate as this one is more active.

@Susurrus: huh?

@recidive: Stupid copy/paste...

#184733: Use named time zones is what I meant.

The code looks good. Thanks for writing tests!

One comment: instead of filling out the admin/settings/date-time page and submitting a form, you should be able to set these variables directly by using $this->drupalVariableSet().

If you can fix that, I'm ready to commit this patch.

As far as additional tests -- it would be great if we could test some of the trickier issues. Writing tests for older timezone related bugs could help. What matters most is that the functionality introduced/fixed by this patch is properly tested.

For the record there is no more drupalVariableSet(), it's now just variable_set(). I also had to fix the logic in another user.test, to deal with the fact that the site-wide time zone no longer defaults to 0.

I would like to point out that we now have access to the date('T') formatter, which is the time zone abbreviation: EST, MDT, etc. We *could* make use of 'T' in various places, for example the ready-made date format options and the time zone select form. But I have not yet made such aesthetic changes since I was making this patch as conservatively as possible (to get it in quicker ;)

In some cases of course the time zone abbreviation is not actually used in the local area, rather it was arbitrarily assigned by the (US-based) time zone database maintainer.

I would caution folks about using the date('T') format unless they are pretty certain of a US audience. I've currently got an issue open with a backup software company because they slap 'EST' onto their email log entries. That's correct, we're using EST, but this is *Australian* EST, currently GMT+10. Once it hits the mail system it gets translated to New York time, also known as EST, but GMT-5(?).

Use the long form instead.

Chasing HEAD.

Chasing HEAD.

The code looks good, but I'm not convinced that the tests are complete. I'd like to get your take on this and/or clarify/extend the test case.

There are actually 3 timezones at work here, not? (i) the timezone of the user submitting the content, (ii) the timezone of the server/database, and (iii) the timezone of the user visiting the content? As far as I can see, only (ii) and (iii) are being tested.

When you say "(i) the timezone of the user submitting the content" are you referring to the node form's "Authored on" field? I could add tests that the correct default value is displayed and that the posted value is stored correctly.

This looks like a pretty good first step. A couple observations/comments:

1) The attempt to detect the timezone name is nice, but it's hard to tell how well it will work since it will be hard to test. It might help to leave a watchdog entry for each detected date string showing the complete date string and the zone it was interpreted into to have a way to debug and test how accurate this is. That could be removed later, once we are confident it is working reliably, or at least that it is coming up with logical fallbacks when there is not enough info. Can we test the date string interpretation using simpletests? If so, we need a variety of browser examples to test, which could be collected in watchdog entries.

2) I tried to created a javascript timezone detector in the Date module and found some browsers that don't have the timezone abbr in the date string but instead have confusing information like 'GMT-0500' which should not be interpreted as 'GMT', and three letter month and day names (which my regex tried to interpret as timezone abbreviations), so we need to be sure the code can handle those situations correctly.

3) I'd like to see us get rid of interpreting a timestamp in format_date() and use real datetime fields instead, but I guess that is a matter for a different patch.

I'm planning to add the javascript timezone detection and maybe some other parts of this patch to the Date API for 5.x and 6.x which will give it a wider audience to help test it.

Oh, one more thing. We need translations of the timezone names. I've adopted a method that killes created in the Event module of listing all the possible timezones in the install file, just so the translator can find them. Since it's in the install file, it doesn't create much overhead. Would it be a good idea to do this in core? It looks like:

/**
 * These strings exist only for the extractor to pick them up and make them
 * available for translation. Putting them here keeps them from being parsed
 * on normal pages.
 *
 * Idea borrowed from Event module. Thanks killes!
 *
t('Africa/Algiers')
t('Africa/Asmera')
t('Africa/Bangui')
t('Africa/Blantyre')
t('Africa/Brazzaville')
t('Africa/Bujumbura')
....

A couple more ideas so I don't lose track of them:

1) When we change the site timezone name in the update, we should display a message saying something like, 'The site timezone name has been set to America/Chicago', just in case the wrong timezone got chosen. It's a tricky business trying to figure out what the timezone should be and it will sometimes fail, and a user message will give the admin a way to immediately see that the wrong choice was made.

2) Trying to update the user timezones all at once with no more info than their offset will be tricky and I suspect there will be a lot of cases where it is not correct. We could display a message to the user the first time they login after this change notifying them about what timezone their account has been set to, with a link to the place where they can fix it if it's wrong.

I can roll a patch with these changes if they sound good, but I thought I'd toss them out on the table first to see if there is interest.

1) I can add some watchdog() calls in the user_timezone callback. Part of your proposal will require passing the full date string via ajax, whereas I'm now only passing the abbreviation (if any) and the calculated parameters. But this seems worth it for debugging. I can't think of a way to meaningfully test the javascript time zone detection via simpletest since it does involve javascript execution; we'd have to use selenium.

2) I'm only looking for between 3 and 5 letters enclosed in parentheses, which in my testing was always a time zone. I should clarify this in the code comments.

3) I don't think I have the patience to do that in this patch but I'll help with a future patch :)

#170: I can add this, if desired. The timezone database is updated quite frequently, every month or so, so this could quickly fall out of date. PHP 5.2.6 has the most recent updates from March 24th: Saigon changed to Ho_Chi_Minh, Calcutta changed to Kolkata, among others. For anyone working on translations, a lot of the work has already been done; see the Unicode Common Locale Data Repository. And according to http://www.twinsun.com/tz/tz-link.htm "ICU contains a mechanism for using this data" although I'm not sure how one would extract all the time zone names in a particular language.

#171: Sounds good to me. Let me know if you're rolling a patch so we don't cross-post or duplicate efforts..

2) Trying to update the user timezones all at once with no more info than their offset will be tricky and I suspect there will be a lot of cases where it is not correct. We could display a message to the user the first time they login after this change notifying them about what timezone their account has been set to, with a link to the place where they can fix it if it's wrong.

We may be likely to have less complaints (overall) if we set all the users to the new site timezone and as you say let them know this has been done, or give the admin an upgrade option to let them set a default. At least for regional and local communities/sites the server zone will be right more of the time than not. We could also probe what's set for Date/Event if either of these are installed, but I'm aware of the issues we've been discussing elsewhere which might make this a bit tricky. It would get more things right when one of these is on the system. Global communities not using Date/Event are going to be messy pure and simple. It's basically a wild guess based around an approximate longitude and I figure more than 50% correct will be pure luck.

In the current version of the patch, the update script will at least give the user the same offset they previously had configured. To me this is "good enough." Even if it's the wrong geographic time zone, at least they have the correct local time.

Of course a human will do a better job bulk-updating the time zones, and we could even make a fancy UI to do it. But there isn't really any place to put this in the Drupal update scripts. So maybe it could be a contrib module?

Other ideas for related contrib modules would include a map-based time zone picker as seen on most operating systems.

I haven't merged this in yet but for future reference, here's a build script to generate the time zone translation list.

This version logs a watchdog message each time the javascript tz detection runs. This is basically debug code that could be removed later.

@mfb, if you have time to roll a patch for #171, go ahead and do it. #175 looks like a good method for handling the translations. The translations will need to be updated from time to time any way we do this.

@macgirvin, I agree we should try to capture anything we can from Date/Event because it will be far more accurate than any guessing we could do. The patch already takes advantage of Date timezone info if it exists, we need to update it to detect Event timezone info. If the Event module adds a function to return the site and user timezone names, we can use that, otherwise the event module uses a zid and has a table that tracks which zid is which zone name, so we'd need to check for the zid and query that table, and then do a str_replace(' ', '_', $zone) on the result to replace spaces with underscores.

On #173, I also thought about just defaulting everyone else to the site timezone. Although it will often be wrong, it will often be right and that's going to be the default value anyway. Using any timezone that has the same offset means someone in Brazil could end up with America/New_York, which is wrong enough to create problems.

The one other idea I have for defaults is check for Date/Event info and use it if available, then update to the site timezone if their offset is the same as the site, otherwise set the value to NULL and wait until they log in the first time to update their values. (This means we'll lose the original offset info, for better or worse.) At that point we'll have their browser info to help make an intelligent guess. Then use our javascript detection in the user edit form as well as the registration form to try to default them to the right value.

@KarenS: Perfect. Given the circumstances I don't think we can do much better. I'm in full agreement that as a last resort, we're better off with NULL and losing data (the old offset) than having the Kurds show up in Baghdad. The js should make everything right for most folks and leave a manageable number of edge cases to mop up afterwards. Since these will mostly be the tier 2 and 3 browsers, for better or worse they're already accustomed to being edge cases.

By the way, the only case where javascript time zone detection works much better than the update script is for Mac and Linux users, which pass in a time zone abbreviation. Windows users can still get assigned to another time zone with the same offset, because there's no abbreviation to work with.

I'll take a look at Event module. Although it does seem odd to incorporate so much contrib module functionality into core. Couldn't Event module have its own update script to migrate the zid data..

#179 - the difference between doing this in the update and when the user logs in is that in the update we will just assign a timezone to them, right or wrong, and the user may not know or notice that it is wrong. If we do it when they log in, I'm assuming the method will *not* be to automatically assign anything, but to direct them to update their timezone with a link to where to do it, and then use the javascript to come up with a sane default for the timezone selector. Since they're on the timezone selection form, they can see what is selected and change it right there if it's wrong.

#180 - this is unusual, but this is an unusual case where we have two widely-used contrib modules that have much better information than we can get in any other way. We could expect both Date and Event to do their own updates of this info, but their updates won't happen until after the core updates finish and those modules are re-installed and updated, which is quite a bit later in the update process and it leaves the site and its users without correct timezone information until that gets done, and that is a critical bit of information for the proper operation of the site. Dries may or may not agree with this, but I think this is one of the few legitimate use cases for updating core from contrib info.

Just noticed that I'm still assigned to this issue, but mfb is doing all the work, so I'm unassigning myself :)

OK I was thinking of you as the "supervisor" on this :)
I can work the NULL + javascript method of migration into the next version. I guess I will assume that we always want javascript time zone detection to run if the user's time zone is NULL. I'm not yet sure how the logic to direct them to their edit page will work, perhaps compare timestamps of last login and when the update script ran.

I think all we need is to display a message when the user logs in and the timezone is NULL. The message could contain a link to the user edit page where they can update the timezone or better yet use drupal_goto() to take them straight to the form and show them a message there.

I think my vote is to show them the message one time, the next time they log in after the site is updated. If they don't respond to that message by doing something with the timezone form, silently set it to the same value a new user would get (the site default) and don't show any more messages. That warns them that the timezone has changed, gives them a chance to do something with it if they care, and gives them a sane default value set if they don't.

If they don't click "Save" on the user edit page, their time zone will still be NULL. when you say "silently set it to the same value a new user would get" I guess you mean the ajax call should forcibly set the time zone? Maybe I'll figure this out when I actually start working on the patch..

Because the user login block passes in a destination, I can't use drupal_goto(). I have to do something like $_REQUEST['destination'] = ('user/' . $user->uid . '/edit'); Also there are various places where the message & redirect logic could go, e.g. system_user('login') hook already has time zone related stuff.

I'd still need to figure out this part: "If they don't respond to that message by doing something with the timezone form, silently set it to the same value a new user would get (the site default) and don't show any more messages." Currently I'm using a getJSON request, but I shouldn't use that to modify user data, I'd have to use a post and make sure I avoid XSRF.

Do you want to try this out? It displays a message each time a user logs in with a NULL time zone, until they set it. Seems much simpler to implement this way. Also it means a user could ignore it and be reminded again later.

Dang - this is looking good. I'm seeing the deficiencies of timezone_name_from_abbr() which put my test account in Australia/Melbourne on a Windows box and Australia/ACT (Canberra) on Linux - both Firefox. Abbreviations were AEST and EST appropriately. Still quite impressive even though it was wrong in both cases.

I tested the upgrade path on a pretty much virgin HEAD system with no add on modules and the upgrade worked cleanly.

I'm just wondering about having the autodetect run at rego (oops registration) time vs. just leaving it NULL (or site_default) for new accounts until first login when there's some opportunity to interact and make it right. As it is, I created a new account and the autodetect was applied without user notice (though the event was logged), which means that since it has been set, logging in after registration doesn't tell you that you might want to check it.

If it is set to NULL on registration, first login would then always take you to the profile edit, which is common on many sites and I don't think is bad but some folks might have issue with. Giving new folks the site default wouldn't have this behavior but might be an improvement over starting them off with something that's still a guess, no matter how good it is.

Often the first thing most folks do after they sign up is to visit their profile so I can live with it no matter how it's done. But I can also foresee future issues about new accounts being given something that is quite possibly wrong by default, so this is as good a time as any to figure out the most desirable behavior.

I don't feel too strongly either way. We could even make it a configurable option -- silently guess the time zone during registration vs. nag the user.

I've added some more to this and re-rolled it. Here's what I changed:

The original patch checked for empty timezone names in the system module and set a message if it wasn't. I removed that code and moved the test to the user module in the login submit function where I have access to the $form_state so I can alter the redirection.

When an existing user logs in, we check if we have configurable timezones and if the timezone is set. If not, they get re-directed to the user edit page where they see a message that they need to set their timezone. Our existing users will see this until they set their timezones and then no more.

When a new user registers, they get their login info in the mail and they can go to the site and login. There is no timezone set yet, so they immediately get redirected to the user edit page with a message. (This also lets them see the other things on the user edit page that they can set).

There is nothing in the code yet that will default new users to any value, it will be empty, so they will always get sent to the edit page to select a timezone when they first login. If anyone feels strongly that that is not the right procedure, we could add a configuration option about whether new users will get set to the site timezone or asked to set their own timezones.

I also added in some code to try to get timezone info from the Event tables, if available. I don't have Event installed to try that part out, so that needs to be tested, but I'm pretty sure it's going to produce the right results.

@KarenS - Hate to be the bearer of bad news, but #189 patch didn't work as advertised. I had exactly the same results as #186 (autodetected timezone on new registration and prior to login that was wrong [although close, same results] both times). The only difference on my end between this and the last report (#187) is that this was a fresh install on HEAD so I did not check any upgrade paths - although I should note that user/1 was redirected to set the timezone as soon as the installation completed.

New users still get the autodetect which is set during the registration process (I checked the DB before attempting a login with the new account) - so in fact it isn't empty when they login the first time as you described.

[I'm also not receiving any registration emails and I've tried a few different addrs. I thought I should flag it in case it is indicative of a regression either in #186,#189, or current HEAD. Haven't had time to fully investigate this yet.]

heh it did take some working around re: login destination ;) I'll look at this some more in the morning..

Ah! One problem is that I left some garbage in the patch from when I was playing around with different options. I'll roll another and post it shortly.

Not receiving mail is odd. There is nothing here that should interfere with email.

Also, macgirvin, you use Event, don't you, can you test the new code there to be sure it picks up your Event timezone info?

Here's another stab at this. The more I thought about the auto redirect the more I got worried that it may have unintended consequences for things like remote logins, so I went back to the original method of just displaying a message to the user and not trying to do any redirection.

We have some timezone handling in the user module and some in the system module, and there's no real consistency (this is a problem in the original code, not this patch). For instance timezone info is added to the user registration form by the user module but added to the user edit form by the system module, and the system module, which defines what the timezones are, was using user javascript to detect the system timezone.

It looks to me like the system module is the logical 'owner' of timezone info, so I consolidated all timezone handling there. I also added an option in the date-time admin form to set a default time zone value for new registrations to be the site timezone, empty, or a user selection, then altered the registration form to match that choice.

I also did a bit of cleanup on the messages which were made up of a bunch of fragments that might be hard to translate and reworked them into complete sentences.

I have not tested the upgrade path or the attempt to get timezone info from the Event module.

I have several suggestions regarding the update process for an individual user's timezone setting:

1. With the patch in #189, am I right in saying that unless the upgrade script manages to retrieve reliable timezone information from the Date or Event modules, then it will set the user's timezone to NULL? Please confirm that this is the case. All users should be prompted to verify their timezone after logging in, whether or not the auto-detect JS succeeds in guessing it. And we should not actually be saving the suggestion that the JS provides, we should only be setting it to the default form selection value. There's no point in more than a mere suggestion, for data as unreliable as that.
2. Personally, my vote is for the timezone detection JS to ignore everything except for proper timezone information, in the form 'Country/Region' or 'Country/City'. The abbreviations are extremely unreliable (as has been pointed out by other people, with the example of 'EST' meaning both US and Australian 'Eastern Standard Time', among others). Offset information would be better off ignored, as each offset corresponds to numerous actual time zones, and what's the chance that we'll randomly guess the right one for a given user? Even with DST info tacked on (which is itself also an unpredictable guess), the odds are against us. If we're ignoring the user's existing time offset setting, then it seems silly to then go and use the offset that their browser client has provided. What I'm saying is, we should either (a) guess the user's timezone if we've managed to get very reliable-looking client-side data, or (b) not guess it at all. As the saying goes, GIGO ('Garbage In, Garbage Out'). And GIGO is not the Drupal way.
3. Also, the watchdog message being logged in user_timezone() should be recording the user to whom it applies.

About the issue of translating the timezones. The build script posted in #175 looks great. However, we'll have to document that the static list of translatable timezones should only be rebuilt for each major Drupal release, and not at any other time. Translatable strings should not be changed after a string freeze, nor once they're in a stable branch. We can't have translatable strings that are getting pumped straight into Drupal's codebase from a third party (i.e. from the PHP library), without a mechanism for controlling exactly when that pumping does and does not happen.

So, for example: if we declare a string freeze for Drupal 7 in (say) 6 months' time, and the PHP team decide to update their timezone list in 7 months' time, then the updated list will just have to wait until Drupal 8.

#194 - yes the latest patch will only use the js detection on a timezone selection form to help make an intelligent guess for a default value, it will not update that value anywhere else. The current process is to try to get the user timezone from Date/Event, otherwise set it to NULL.

We have to use the timezone abbreviation for the detection because 1) as far as I know, no browser puts the timezone name you're looking for into its date string -- it will either be the abbreviation or something like 'Central Daylight Savings Time', and 2) the abbreviation is what the timezone_name_from_abbr() function uses and there is no other way in PHP 5.2 to search for a match. This is good enough to set a default value, but as we said above, not good enough to use as an automatic selection.

#195 - good point, so we might as well put the hard-coded values in there instead of using the function so we can see what they are. I've updated the patch to add that in, along with a TODO to confirm those values right before string freeze.

I also added in an option whether or not you want users with empty timezones to be reminded on login. It will default to turn that option on in the update if we set user time zones to NULL, but this way the admin can control that behavior. And I added a message to the update to tell the admin that there are now new timezone options they can set.

Here's another patch, with those changes and a couple small bug-fixes.

The idea of the build script was to use it to regenerate the list of time zones in future drupal releases. It could live in the scripts directory or we could hunt it down here.

One problem I found so far: In your Event module update script, the name column is ambiguous, you need to specify event_timezones name.

Also you used the t() function in the update script. I didn't use it because none of the other update scripts seem to use it when calling drupal_set_message(). I'm not sure what the deal is, as I didn't manage to find any documentation on using the t() function in hook_update_N().

I tried out the patch and it looks like system.js is not loaded on the user/edit page.

t() in update makes no sense, while the po strings are imported afterwards.

I resolved the above issues and made some other tweaks including: time zone strings in system.install were apparently not from the latest version of PHP so they were already out of date; on installation admins were presented with a nice "Please set your user account time zone" so the empty time zone message now defaults to off; date_timezone() renamed to system_timezone().

I wouldn't say I am completely happy with the new setup; on my sites I would still like to convert user's time zones on upgrade and use js time zone detection on registration, rather than forcing users to set their time zones in both cases. But I won't argue on these minor points if others think it's working well.

Chasing head.

Still applies with offset. All tests pass.

I noticed a slight delay when viewing the timezone on user/1/edit - it showed Africa/Abidjan initially, then switched to London very quickly (yay!). Was this because it was the first time visiting my account settings? If so it might be nice to add this to the user/1 settings on site installation.

It's a shame about the huge list of country strings, but iirc (without reading through the 200 previous posts), there's no way around this right?

Other than that, the patch looks good to me, although no time for an in-depth review.

@catch: this is how it started working in recent versions of the patch. In previous versions there was no javascript timezone detection on the user-edit page, only the user-register and user-create pages. I don't mind the "old" way at all. I am basically going with whatever folks here suggest for these "UI details" so as to get the guts of patch committed.

It would be possible to filter some more obsolete timezones from the list, if we want to maintain a list of obsolete timezones in the code. For example, in recent versions of the timezone database, Asia/Kolkata replaced Asia/Calcutta. PHP timezone_identifiers_list() lists all the timezones, old and new, for backwards compatibility. My thought was this backwards compatibility will also be useful for drupal sites, so the selection shows up if a user had previously specified "Asia/Calcutta" according to the old database.

I don't think "old" timezones are needed in Drupal. Drupal 7 will (hopefully :)) be the first release with proper timezones in it, so it won't need to be aware of timezones predating its release date. (I'd also like to think that Drupal 8 and onwards would be able to omit timezones obsoleted by their release, and have the update script change the obsoleted time zones to their proper, current (at the time) ones.)

But perhaps it would be a good thing if it was possible to extend the list of timezones through a contrib module or otherwise? That way, if someone really needs the obsolete timezones, they can still add them. And likewise, if someone needs a timezone that's being introduced after, e.g., the release of Drupal 7 but before Drupal 8, they can still include it on their site. (Not that this necessarily has to be part of this patch, just something that might be nice to keep in mind. :))

I had to register on a vbulletin board the other day, and it had the normal timezone options, then 'automatically detect daylight saving' as a checkbox. That'd be a nicer UI IMO, but no reason to hold this patch up with it - since it'd be either an easy followup patch or contrib option.

@Freso: In that case this patch also has a reason to update timezones, as it migrates timezones from existing installations of the date or event modules. There are 107 obsolete timezones in the tzdata "backward" file which could be used to create an update script.

Excluding old timezones would require adding an array of ~46 obsolete timezones to this patch. With this additional filtering, _system_zonelist() would currently list 401 timezones instead of 447.

@catch: I'm not sure I understand what the vbulletin UI is doing. If a user is selecting their daylight saving status rather than the site figuring it out for them, that would imply that we don't really know their timezone. The goal here is for details like daylight saving time to be taken care of automatically by feeding the user's geographic timezone to PHP.

As far as UI the main improvement I could think of would be just to copy the clickable map used by most operating systems.

About translation of timezone names... I finally downloaded the XML files from unicode.org (the Unicode Common Locale Data Repository that I mentioned earlier in this thread) to take a look at how it works.

In an XML file for each language, e.g. es.xml, each timezone is mapped to a translated form of the exemplar city, e.g. <zone type="Africa/Johannesburg"><exemplarCity>Johannesburgo</exemplarCity></zone>

There is also supplemental data that maps each timezone to a metazone, for example America/New_York and America/Toronto map to "America_Eastern". And these metazones are also translated in the language files, e.g. in es.xml as "Hora oriental". However metazone mappings change over time, e.g. <timezone type="America/Santo_Domingo"><usesMetazone to="1974-10-27 05:00" mzone="Dominican"/><usesMetazone to="2000-10-29 06:00" from="1974-10-27 05:00" mzone="Atlantic"/><usesMetazone to="2000-12-03 06:00" from="2000-10-29 06:00" mzone="America_Eastern"/><usesMetazone from="2000-12-03 06:00" mzone="Atlantic"/></timezone> so these aren't such a good idea to use.

Patch cannot be applied to HEAD.

Chasing HEAD.

Btw would it be possible for testing.drupal.org to automatically inform me when patches no longer apply..?

@mfb : http://drupal.org/project/issues/subscribe-mail/drupal : "Receive notification of failed patch tests."

... but last patch of active issue need to be test frequently.

@lilou I've never received a failed patch test. Maybe it only tests once as soon as a patch is uploaded.

mfb: it doesn't quite work yet - http://groups.drupal.org/node/13990

Chasing HEAD.

sub

Chasing HEAD.

Chasing HEAD.

Chasing HEAD (you people and your time() REQUEST_TIME craziness ;)

User time zone tests : 26 passes, 0 fails, 0 exceptions

Patch applies but update user_update_7002() can not be run. I get
An error occurred. http://www.example.nl/update.php?id=2&op=do <br /> <b>Fatal error</b>: Call to undefined method DatabaseSchema_mysql::escapeTable() in <b>/var/www/home/d7/includes/database/mysql/schema.inc</b> on line <b>22</b><br />
This was on a clean HEAD install.

Besides that, this module should use the new DBTNG database calls: see http://drupal.org/node/224333#dbtng

Actually that's a bug in the db layer, I filed a new issue here: #310607: Fatal error: undefined method DatabaseSchema_mysql::escapeTable()

I agree that the database calls in the .install files should get an overhaul (e.g. using foreach to iterate over the results) but I *think* this is at least working and it would be good to get further reviews. I will revisit when the above issue is fixed.

Chasing HEAD, and a couple tweaks related to new db layer.

I'm going to chime in here and say I'd really like to see this in D7.

I'm also going to say that unfortunately I don't have time to review it now, but it'd be great to get some eyes on this so that when I do have time (probably next week) it was all nice and nitpicked and ready to commit. ;)

I would like to test this, but can't get to it immediately. Marking it so I remember to get back to it :)

Chasing HEAD -- revamp system_user_*() hooks.

Incredibly minor, but I notice that most of the code comments refer to "time zone" and "time zones", while some refer to "timezone" and "timezones".

OK I changed ~6 cases to "time zone", let me know if I missed any.

we should put this in the patch spotlight: http://drupal.org/node/132970

it'll need a description written up and a list of things to test.

Here's a super-abbreviated start. If I or someone has more time it would be possible to come up with a very long list of detailed test scenarios.

#11077: Introduce Daylight Saving Time for Drupal revamps Drupal's support for system default and user-configured time zones to use PHP 5's time zone subsystem. With this patch, time zones will be identified by geographic names such as "Europe/London" rather than numeric offsets from GMT. This is the same method that virtually all operating systems use to identify time zones and perform date/time calculations involving non-GMT time. The offsets that Drupal has used in the past are wrong literally half the time due to daylight saving (summer) time and have to be manually updated like clocks -- but even when set correctly, times cannot be calculated correctly six months in the past or future.

For more on date/time in PHP 5, see http://www.php.net/manual/en/book.datetime.php

The current version of the patch includes some changes and additions beyond the time zone revamp (see the issue comments for discussion on these items):

• Some time zone-related code is moved from user module to system module;
• The user time zone can now be set automatically via javascript on the registration page or the user edit page, and can now appear as a visible field on the registration page so that it can be manually verified (due to difficulty in 100% accurate detection of the user's time zone in some environments like Internet Explorer);
• The automatic javascript time zone detection is logged for debugging purposes;
• Time zones configured by the Date or Event modules on Drupal 6 are automatically migrated to Drupal 7 by update scripts in system.install and user.install;
• All user-configured time zone settings will be wiped out by the update script and will have to be reconfigured by users, because it is impossible to migrate time zone offsets to time zone names accurately (dozens of time zones may share the same offset at various times of year);
• Time zone-related tests have been added to system.test and user.test; and
• A list of time zone names is included in system.install for purposes of seeding the translatable string database.

Things to review:

• Upgrade from Drupal 6 to Drupal 7: Check the messages displayed during the update script, test the attempt to migrate the system default time zone, and review the potential disruption of user time zones being wiped out;
• Upgrade from Drupal 6 to Drupal 7 with Date or Event modules installed: Test the migration of user-configured time zones;
• Test all the combinations of time zone configurations at admin/settings/date-time;
• Test functionality of format_date(), i.e. all display of dates and times;
• Test user time zone functionality at registration, user login and user edit;
• Review the removal of the option of having both a time zone hidden field and automatic time zone detection at registration (with this patch it is possible to have EITHER a hidden time zone field OR automatic javascript time zone detection but NOT BOTH -- some may want to restore this option);
• Review the new time zone-related System and User tests, and suggest any ideas for additional tests;
• Review the UI for configuring time zones, including a very long select form, and consider potential for improving the UI and/or ensuring that add-on modules are able to improve the UI (e.g., would it be useful to be able to drupal_alter() the list of time zones); and
• Review feasibility of translating time zone identifiers, ideally using pre-existing translations in the Unicode Common Locale Data Repository.

[Edited to clarify the present inability to have BOTH a *hidden* time zone field and automatic javascript time zone detection on the register page -- admins have to choose one or the other].

I just looked at admin/settings/date-time and I'm not sure the fieldsets (Locale and Formatting) are totally logical. Maybe the time zone options should go in to a separate "User-configurable time zones" fieldset. The difference between "locale" and "formatting" is not entirely clear to me either.

Chasing HEAD and fix a regression in #226: some code needs to be shared between system_user_form() and system_user_register(). Per above I added an extra fieldset for "User-configurable time zones".

@mfb: Just so you're aware, we're in #drupal prepping for UNSTABLE-2 which should be tagged either later tonight or tomorrow morning so patches are getting committed every few mins. So I'd hold off doing more re-rolling until tomorrow for your own sanity. :)

Chasing HEAD, and finally fixed a bug I reported way back in #105.

Installed a clean version of Drupal 7 from HEAD.
Patch still applies against HEAD, with a few offsets.
The need for a schema update was properly detected, and two were applied successfully.
My local timezone was correctly identified as the suggested value for the system default time zone.
The reminder prompt for a user without a confirmed time zone setting to set their own time zone worked.
The user time zone detection in the self-update context worked.
The options around user timezone handling for new registrations worked as described.

One issue I noticed is that when an admin views a new user registration that has an empty timezone assignment, the timezone is not shown as 'not selected' but instead shows as (I believe) the detected timezone of the admin. So, the person administering the new user can't distinguish between users with no time zone and users whose time zone is set to their own time zone. I believe that means changing anything else on the user and saving the result will silently change the user's time zone from none to the admin's.

I think it would be preferable if the admin could distinguish between a user whose time zone is set from one whose time zone is not set, and default to admin edits not changing a null time zone.
--
Joe Kyle
--jjkd--

Thanks for catching that. so we need to allow the form to preserve a NULL timezone, and only do javascript time zone detection when the user is editing her/his own account. I think it would be ok to add an empty-string option to the form, allowing also non-admin users to set their time zone to undefined.

OK to address #244, I added a $blank param to _system_zonelist(), which prepends a "None selected" option to the zonelist. One other thing I noticed, since javascript auto-detection was removed from the hidden timezone field, I changed this hidden field to use #value rather than #default_value.

Ok, went through the process again, editing a user from the admin context is much improved.

The only remaining wrinkle in this category I noticed is when a user is editing their own time zone, and selects the 'none' option, auto-detection runs again when the page reloads, and so the 'none' option is not shown as the user's current setting. The value in the db is correctly empty at this point.
--
Joe Kyle
--jjkd--

I wouldn't consider this a problem. Do we need to make it easy for users to maintain an empty time zone setting? Previously I don't think Drupal has allowed users to set an empty time zone. And we would have to differentiate somehow between a missing time zone setting and an intentionally empty time zone setting.

Yes, I think you are right. I agree, allowing the user to change to an empty time zone setting is probably not desireable. So, if we can eliminate the "none selected" option only in the self-edit context, I think that would help.

In working through this in my head, I think what was going on there may be a consideration here for a potential usability issue. It appears to me that we are 'overloading' the time zone field, and really using it for two different purposes when displaying a value:

1) If the user's time zone is set in the database, display the current database setting
2) If the user's time zone is not set in the database, display the results of the time zone detect code

In the case of #1, the user can safely hit either 'save' or navigate away from the page, and whatever they saw as their 'current' time zone setting is retained in either case. If they change the time zone value, the need to hit 'save' is a reasonable expectation.

In the case of #2, navigating away from the page will discard the time zone value displayed, and the empty database value will remain. There really isn't an indication to the user that the value shown is a 'recommended setting', and they need to hit 'save' to retain it. The user may interpret the display as their 'current' time zone setting, and not realize that it must be saved at that point.

An example use case would be: System is configured to create new users with empty time zone values, and to prompt users for time zone entry. User logs in, sees the prompt, clicks it to go (or otherwise eventually navigates) to their user profile. They see a time zone displayed, and thinks their time zone is set, so they navigate away, This repeats each time they log in, and they get the impression that this is something unreliable in the system. Eventually change something else in their profile, and now the time zone setting does persist.

I would suggest that in the case of #2, we provide some indication to the user that the value displayed is a recommended time zone value, and they need to save it.
--
Joe Kyle
--jjkd--

solution for option #2:
drupal_set_message('Timezone will be automatically detected if possible, please edit it if found incorrect, hit save button to confirm selection.');

There's a problem with using drupal_set_message(): apparently the system_user_form() hook fires when the form is posted, but the submitted form values are not passed thru in $edit. So, after a user sets a time zone successfully, she/he will see a message that was set during form submission (not displayed at that time because it was a POST request) about needing to set a time zone.

Maybe the only way I can do this cleanly in system_user_form() is to render the message in the form itself (like in the field description).

OK to address #249/#250 I added a message in the description of the timezone fieldset.

Great, thanks, I think that pretty much covers #249/#250.

The only other thing I've noted is that the field description rendered in both the admin and self-edit contexts is the same, and is currently:

"Select your current local time. Dates and times throughout this site will be displayed using this time zone."

I would suggest that there be different versions for the self-edit and admin contexts, perhaps:

"Select your current local time. Dates and times throughout this site will be displayed to you using this time zone."

and

"Select the desired time zone for this user. Dates and times throughout this site will be displayed to the user using this time zone."

Has anyone complained about this issue before? Because it's been like this for ages, and I don't think this is the only place in core where site admins see a "you" that actually refers to the user in question, see also: "A valid e-mail address. All e-mails from the system will be sent to this address. The e-mail address is not made public and will only be used if you wish to receive a new password or wish to receive certain news or notifications by e-mail." If this is in fact considered a usability problem then I would suggest to change it to some neutral wording rather than adding extra strings.

Sorry, if that's the norm, I would agree that it isn't the responsibility of this patch to change that 'standard'. We can probably leave it to those concentrating on usability to decide if a core-wide issue should be opened on that.

How about the fairly neutral:

"Select the desired local time and time zone. Dates and times throughout this site will be displayed using this time zone."

+1 to neutral statement!

Here is a version of patch 29 with the field description changed to the more neutral version. Full credit to Mark, all I did was change the one line.