Moodle

DST not applied properly when requesting explicit timezones

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.8.4, 1.9
  • Fix Version/s: 1.8.5, 1.9.1
  • Component/s: General, Libraries
  • Labels:
    None
  • Environment:
    Any, with timezones loaded...
  • Affected Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE

Description

Some days ago, with MDL-13959 , we fixed the usergetdate() to work properly with timezones passed as parameters, because DSTs weren't being properly calculated then. The solution was about to "save" the parameter value and then pass it straight to the dst_offset_on() function. Else, the original parameter became transformed and DST weren't applied.

The same behaviour can be extended to other moodlelib functions:

make_timestamp()
userdate()
usergetmidnight()

All them can accept one timezone parameter and, if used, DST aren't properly applied.

IMPORTANT NOTE: This doesn't affect normal Moodle operations, where the DST to be applied is calculated by looking server and user timezones. This ONLY affects it we want to know the time in another timezone (imagine one block showing different times or something like that).

So those functions should be patched with a similar solution that the one used for usergetdate() in MDL-13959.

Attachments

  1. Text File
    moodlelib.php.patch
    29/Mar/08 12:55 PM
    2 kB
    Eloy Lafuente (stronk7)
  2. Text File
    test.php
    29/Mar/08 12:27 PM
    3 kB
    Eloy Lafuente (stronk7)

Issue Links

This issue has a non-specific relationship to:
MDL-13959 Timezone Problem Critical Closed
This issue has been marked as being related by:
MDL-14158 DST changes not being applied on time Minor Open

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Eloy Lafuente (stronk7) added a comment - - edited

Attaching one test to demonstrate how userdate() and usergetmidnight() fails when a custom timezone is passed. Results are this (with local clock set at 00:00:00):

Execution details:
Without tz, calculations are for timezone "Australia/Perth"
With tz, calculations are for timezone "Europe/Madrid"

Now: (1206745200)
Now (usergetdate-notz): 29-03-2008 08:00
Now (usergetdate-withtz): 29-03-2008 00:00
Now (userdate-notz): sábado, 29 de marzo de 2008, 08:00
Now (userdate-withtz): sábado, 29 de marzo de 2008, 01:00
Midnight (userdate-notz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206716400)
Midnight (userdate-withtz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206741600)

In 7 days (7*24*60*60 seconds): (1207350000)
7 (usergetdate-notz): 05-04-2008 07:00
7 (usergetdate-withtz): 05-04-2008 01:00
7 (userdate-notz): sábado, 5 de abril de 2008, 07:00
7 (userdate-withtz): sábado, 5 de abril de 2008, 00:00
Midnight (userdate-notz): sábado, 5 de abril de 2008, 00:00 (mid: 1207324800)
Midnight (userdate-withtz): sábado, 5 de abril de 2008, 00:00 (mid: 1207350000)

Show
Eloy Lafuente (stronk7) added a comment - - edited Attaching one test to demonstrate how userdate() and usergetmidnight() fails when a custom timezone is passed. Results are this (with local clock set at 00:00:00): Execution details: Without tz, calculations are for timezone "Australia/Perth" With tz, calculations are for timezone "Europe/Madrid" Now: (1206745200) Now (usergetdate-notz): 29-03-2008 08:00 Now (usergetdate-withtz): 29-03-2008 00:00 Now (userdate-notz): sábado, 29 de marzo de 2008, 08:00 Now (userdate-withtz): sábado, 29 de marzo de 2008, 01:00 Midnight (userdate-notz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206716400) Midnight (userdate-withtz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206741600) In 7 days (7*24*60*60 seconds): (1207350000) 7 (usergetdate-notz): 05-04-2008 07:00 7 (usergetdate-withtz): 05-04-2008 01:00 7 (userdate-notz): sábado, 5 de abril de 2008, 07:00 7 (userdate-withtz): sábado, 5 de abril de 2008, 00:00 Midnight (userdate-notz): sábado, 5 de abril de 2008, 00:00 (mid: 1207324800) Midnight (userdate-withtz): sábado, 5 de abril de 2008, 00:00 (mid: 1207350000)
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

About the results above,

when notz is passed as parameter, then $CFG->timezone is used (or $USER->timezone if set). And everything works ok there.
when tz is passed as parameter, usergetdate() works ok, returning 00:00:00, but userdate() fails and usergetmidnight() too, returning different timestamp than the execution one (and should be the same).

In the 7 days part, we can see that notz times are also ok (DST ends) but tz times continue failing, they only are correct for usergetdate() but not for userdate() that shows one less hour, and usergetmidnight() (that, with DST in action should be different from the execution timestamp).

Show
Eloy Lafuente (stronk7) added a comment - About the results above, when notz is passed as parameter, then $CFG->timezone is used (or $USER->timezone if set). And everything works ok there. when tz is passed as parameter, usergetdate() works ok, returning 00:00:00, but userdate() fails and usergetmidnight() too, returning different timestamp than the execution one (and should be the same). In the 7 days part, we can see that notz times are also ok (DST ends) but tz times continue failing, they only are correct for usergetdate() but not for userdate() that shows one less hour, and usergetmidnight() (that, with DST in action should be different from the execution timestamp).
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Attaching one patch that applies the fix as talked above...

Show
Eloy Lafuente (stronk7) added a comment - Attaching one patch that applies the fix as talked above...
Hide
Permalink
Eloy Lafuente (stronk7) added a comment - - edited

And here there are the results of the test with the patch applied:

Execution details:
Without tz, calculations are for timezone "Australia/Perth"
With tz, calculations are for timezone "Europe/Madrid"

Now: (1206745200)
Now (usergetdate-notz): 29-03-2008 08:00
Now (usergetdate-withtz): 29-03-2008 00:00
Now (userdate-notz): sábado, 29 de marzo de 2008, 08:00
Now (userdate-withtz): sábado, 29 de marzo de 2008, 00:00
Midnight (userdate-notz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206716400)
Midnight (userdate-withtz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206745200)

In 7 days (7*24*60*60 seconds): (1207350000)
7 (usergetdate-notz): 05-04-2008 07:00
7 (usergetdate-withtz): 05-04-2008 01:00
7 (userdate-notz): sábado, 5 de abril de 2008, 07:00
7 (userdate-withtz): sábado, 5 de abril de 2008, 01:00
Midnight (userdate-notz): sábado, 5 de abril de 2008, 00:00 (mid: 1207324800)
Midnight (userdate-withtz): sábado, 5 de abril de 2008, 00:00 (mid: 1207346400)

Show
Eloy Lafuente (stronk7) added a comment - - edited And here there are the results of the test with the patch applied: Execution details: Without tz, calculations are for timezone "Australia/Perth" With tz, calculations are for timezone "Europe/Madrid" Now: (1206745200) Now (usergetdate-notz): 29-03-2008 08:00 Now (usergetdate-withtz): 29-03-2008 00:00 Now (userdate-notz): sábado, 29 de marzo de 2008, 08:00 Now (userdate-withtz): sábado, 29 de marzo de 2008, 00:00 Midnight (userdate-notz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206716400) Midnight (userdate-withtz): sábado, 29 de marzo de 2008, 00:00 (mid: 1206745200) In 7 days (7*24*60*60 seconds): (1207350000) 7 (usergetdate-notz): 05-04-2008 07:00 7 (usergetdate-withtz): 05-04-2008 01:00 7 (userdate-notz): sábado, 5 de abril de 2008, 07:00 7 (userdate-withtz): sábado, 5 de abril de 2008, 01:00 Midnight (userdate-notz): sábado, 5 de abril de 2008, 00:00 (mid: 1207324800) Midnight (userdate-withtz): sábado, 5 de abril de 2008, 00:00 (mid: 1207346400)
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Note that now both userdate() and usergetmidnight() are ok when invoked with timezone. And results when no timezone is specified (normal Moodle usage) aren't modified at all.

I'd apply this from 1.8 to 2.0. For your consideration.

Show
Eloy Lafuente (stronk7) added a comment - Note that now both userdate() and usergetmidnight() are ok when invoked with timezone. And results when no timezone is specified (normal Moodle usage) aren't modified at all. I'd apply this from 1.8 to 2.0. For your consideration.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Adding some people here to look, test and decide... ciao

Show
Eloy Lafuente (stronk7) added a comment - Adding some people here to look, test and decide... ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Offtopic: These days (Both Australia and Europe) were changing their DSTs.. and I've been executing the test.php above repeatedly.

And I've observed that changes in DST didn't happen on time. In fact the European one was applied some hours BEFORE the expected one and the Australian one was applied some hours later.

Note this comment isn't related with this bug (make DST to work when using explicit timezones).

Perhaps we should open a new bug about that DSTs not being applied on time. Ciao

Show
Eloy Lafuente (stronk7) added a comment - Offtopic: These days (Both Australia and Europe) were changing their DSTs.. and I've been executing the test.php above repeatedly. And I've observed that changes in DST didn't happen on time. In fact the European one was applied some hours BEFORE the expected one and the Australian one was applied some hours later. Note this comment isn't related with this bug (make DST to work when using explicit timezones). Perhaps we should open a new bug about that DSTs not being applied on time. Ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Last chance to comment something about this bug. Will commit tomorrow.

Also, I've created MDL-14158 about DSTs not being applied on time.

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Last chance to comment something about this bug. Will commit tomorrow. Also, I've created MDL-14158 about DSTs not being applied on time. Ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Done, applied from 18_STABLE to HEAD. Ciao

Show
Eloy Lafuente (stronk7) added a comment - Done, applied from 18_STABLE to HEAD. Ciao
Hide
Permalink
Mathieu Petit-Clair added a comment -

I've been testing all sorts of cases, changing time on my computer, etc. Seems to be working fine now.

Show
Mathieu Petit-Clair added a comment - I've been testing all sorts of cases, changing time on my computer, etc. Seems to be working fine now.

People

Vote (0)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.