date_time() is defined as returning the “time() adjusted for the current site and user”. But, depending on the site’s settings, the value returned can have a completely bogus timezone.

Let me quickly include the current definition of date_time():

  global $user;

  if ($offset) {
  	return (time() - date("Z")) + offset;
  }
  elseif (variable_get('configurable_timezones', 1) && $user->uid && strlen($user->timezone)) {
    return (time() - date("Z")) + $user->timezone;
  }
  else {
    return (time() - date("Z")) + variable_get('date_default_timezone', 0);
  }
}

(Let’s ignore the fact that offset is missing a $ in line 4.)

time() returns the current datetime in GMT. date("Z") returns the GMT offset for the timezone of the webserver. So the first thing date_time() does is convert the timestamp to the timezone of the webserver. Buy why?

The $user->timezone and variable_get('date_default_timezone', 0) are both relative to GMT, so first converting it to the webserver’s timezone will result in bogus timezones.

For example, if the server is in New York (UTC-0500) and the site’s timezone is set for Phoenix (UTC-0700), the returned timezone is converted to UTC-0200 which is completely wrong.

With the current version of the date_time(), you will only get a useful value returned if the webserver is the same timezone as the user’s or site’s timezone; i.e. date_time() will return the same value as time(), a GMT timestamp. So while that value is useful, it's still wrong since date_time() is supposed to return the timestamp relative to the user or site timezone.

Now knowing the webserver offset may be useful for some situations, so maybe that code could go into another function. But it's clearly not needed for date_time().

The attached patch:

  1. strips - date("Z") from date_time()
  2. creates a date_get_timezone_offset() which simply returns the configurable timezone offset (i.e the user's timezone offset or the site's.)
  3. modifies date_time() and theme_date_format_interval() to use date_get_timezone_offset()
  4. converts $user->uid && strlen($user->timezone) into the simpler and faster !empty($user->timezone)
  5. changes a tab into two spaces
CommentFileSizeAuthor
date_time.patch2.49 KBJohnAlbin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dark_Athena’s picture

Dark_Athena CreditAttribution: Dark_Athena commented
Component: Code » Documentation
Assigned: Unassigned » Dark_Athena
Category: bug » support
Priority: Critical » Normal
Status: Needs review » Postponed (maintainer needs more info)

I'm not sure how to correctly apply this patch and am wondering if you, or someone else, would be able to give an idea of how to correctly install this. Thank you in advance!

asimmonds’s picture

asimmonds CreditAttribution: asimmonds commented
Component: Documentation » Code
Assigned: Dark_Athena » Unassigned
Category: support » bug
Priority: Normal » Critical
Status: Postponed (maintainer needs more info) » Needs review

@Dark_Athena - Please do not hijack issues with patch-related support requests

Dark_Athena’s picture

Dark_Athena CreditAttribution: Dark_Athena commented

Sorry. Just looking for some help. My apologies. I thought that if someone was going to post a fix, there would be clear directions how to apply said fix.

L3na’s picture

L3na CreditAttribution: L3na commented

Hey Dark_Athena, I was wondering the same thing. I'm a noob! :)
If you search this site for information on patches, you'll find an entry in one of the handbooks on applying them. As for me, I applied the patch manually. I made an off-line version of the latest Date Module (the Dev snapshot) and looked at the patch to see which of the files it was referring to (Date.inc and Date.module). Then I opened both files in wordpad and used Search to delete the entries which are marked with:

-

in the patch file and add entries marked as:

+

in the patch file. Basically reading the code in the same way as the computer would do. Then I uploaded the new code to the server and voila - the bug was fixed! Thanks JohnAlbin for the update!

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

The Drupal handbooks (see the Handbooks link at the very top of this page) are a great place to find information like this. What you want specifically is this: http://drupal.org/patch

L3na, I'm glad the patch worked for you.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Assigned: Unassigned » JohnAlbin
Status: Needs review » Reviewed & tested by the community

Since the patch worked for L3na, I'm going to mark this RTBC.

Karen, I wrote the patch before I saw you were hard at work on Date 2. However, there are many of us using 5.x-1.8 that need a quick fix. It would be nice to get a fix into the DRUPAL-5--1 branch. Let me know if you would like me to rework the patch in any way.

p.s. I'd like to point out that this fixes the Calendar bug where the highlighted "today" can be off by 1 day: http://drupal.org/node/99223

KarenS’s picture

KarenS CreditAttribution: KarenS commented

My only reservation about this is that things that fix this for some people do not fix it for others, so we have gone around several times making changes to this code and still having people come back and say that it doesn't work right for them. I also can see that it does fix things for some people, and there's no question that things are not working right now.

Let me think about it and get back to you.

My real fix for this is Date 5.2, which, so far, seems to get these things right.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

Yes, I've seen the round and round over at http://drupal.org/node/99223, so I understand your hesitancy.

But if we just look at what date_time() is supposed to return (timestamp adjusted for the current site and user), you can see that the returned value is incorrectly calculated. So that needs to be fixed.

The patch file looks a little confusing, but if you look at the unpatched file and the patched file side-by-side, it's a pretty straight-forward change.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

Karen, I'm sure you already knew this. But I now understand why the - date('Z') was used.

For those that are curious, the following 2 pieces of code return the same value (the current datetime in GMT):

  1. gmdate('Y-m-d H:i:s', time() );
  2. date('Y-m-d H:i:s', time() - date('Z'));

So the "proper" timestamp for a local time depends on the function that uses it.

I still think the above patch is part of the right solution (for 5.x-1.x). But we'll also need to check all instances of date_time() in the date and calendar modules to make sure that they are using the returned value correctly (i.e. not using it in date() or in some other way that might cause PHP to apply a server-based offset.) _calendar_setup_form() needs updating, for instance.

I see that date_time() is missing from Date 5.x-2.x. Probably a good thing. :-)

jayp01’s picture

jayp01 CreditAttribution: jayp01 commented

Just one more vote for publishing this patch... I was having the exact same problems and it fixed them. My setup sounded similar to everyone elses... Basically, the calendar was just bumping the current day to "tomorrow" starting late in the evening.

KarenS’s picture

KarenS CreditAttribution: KarenS commented
Status: Reviewed & tested by the community » Closed (won't fix)

I'm not making changes to the 5.1 version. I'm officially recommending you move to the 5.2 version now. If you have problems in that version you can check for existing issues or add new ones. Feature requests are now getting posted to the D6 version to be back-ported to 5.2.