Last modified: 2011-12-09 18:12:27 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T34351, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 32351 - #time and #timel ignore explicitly specified timezone
#time and #timel ignore explicitly specified timezone
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
ParserFunctions (Other open bugs)
unspecified
All All
: Normal major with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-reviewed
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-10 22:15 UTC by Van de Bugger
Modified: 2011-12-09 18:12 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
One line patch. (1.14 KB, patch)
2011-11-10 22:15 UTC, Van de Bugger
Details
Patch v2. (4.27 KB, patch)
2011-11-11 23:42 UTC, Van de Bugger
Details
Patch v3. (4.60 KB, patch)
2011-11-12 08:32 UTC, Van de Bugger
Details
Patch v4. (3.90 KB, patch)
2011-11-21 18:59 UTC, Van de Bugger
Details

Description Van de Bugger 2011-11-10 22:15:14 UTC
Created attachment 9416 [details]
One line patch.

Both #time and #timel ignore explicitly specified timezone. For example:

* {{ #time:  Y-m-d H:i:s | 2011-11-10T23:00 Europe/London  }}
* {{ #time:  Y-m-d H:i:s | 2011-11-10T23:00 Europe/Rome    }}
* {{ #timel: Y-m-d H:i:s | 2011-11-10T23:00 Europe/London  }}
* {{ #timel: Y-m-d H:i:s | 2011-11-10T23:00 Europe/Rome    }}

produces 

* 2011-11-10 23:00:00
* 2011-11-10 23:00:00
* 2011-11-10 23:00:00
* 2011-11-10 23:00:00 

With proposed one-line (not counting comments) patch, both function start to respect explicitly specified timezone. #time converts specified time to UTC, #timel -- to local server time:

* 2011-11-10 23:00:00 // London time == UTC
* 2011-11-10 22:00:00 // 23:00 Paris time == 22:00 UTC
* 2011-11-11 03:00:00 // 23:00 London time == 03:00 next day my local time
* 2011-11-11 02:00:00 // 23:00 Paris time == 02:00 next day my local time
Comment 1 Brion Vibber 2011-11-11 00:40:46 UTC
This would be a good candidate for adding some test cases to parserFunctionsTests.txt
Comment 2 Van de Bugger 2011-11-11 23:42:25 UTC
Created attachment 9424 [details]
Patch v2.

Another attempt to the problem.

This patch implements following:

1. Both #time and #timel recognize and respect explicitly specified timezone:

{{ #time: ... | 2011-11-12 23:00 UTC }}
{{ #time: ... | 2011-11-12 23:00 Europe/Paris }}
{{ #time: ... | 2011-11-12 23:00 Europe/Moscow }}
etc.

2. If timezone is not explicitly specified, UTC is assumed.

3. #time output is UTC:

{{ #time:  Y-m-d H:i | 2011-11-12 23:00 Europe/Paris }} = 2011-11-12 22:00

4. #timel respects user preferences, not wiki default timezone ($wgLocaltimezone; wiki default timezone is used if user is not logged in):

{{ #timel: Y-m-d H:i | 2011-11-12 23:00 UTC }} = 2011-11-13 03:00

(Assumed user preferences is +04:00).

This differs from existing implementation:

1. There is a bug in current implementation. If PHP >= 5.2, explicitly specified timezone is not respected. (However, if PHP < 5.2, it is respected).

2. #timel reports time in wiki default timezone. I changed this behaviour intentionally. I think using user preferences is more useful than using wiki timezone. At least, it is aligned with wiki behaviour: at the bottom of page modification date is shown using user timezone, not wiki timezone.

3. Current implementation uses wiki default timezone if timezone is not explicitly specified... (However, I did not tested it thoroughly.) BTW, there are comments in funcsParserTests.txt:

> # fixme: #time seems to be accepting input as local time, which strikes me as wrong

> Input times should probably be UTC, not local time

Anyway, exact semantic of #time and #timel was never precisely defined. The only mention about timezones is:

> This function is identical to {{#time: ... }}, except that it uses the local
> time of the wiki (as set in $wgLocaltimezone) when no date is given.

----

Brion Vibber wrote:

> This would be a good candidate for adding some test cases to
> parserFunctionsTests.txt

Do you mean funcsParserTests.txt? This patch includes few tests for #time. Problems:

1. There are no tests for #timel, because it depends on user preferences. If user preferences are unknown, it is not possible to specify result.

2. It is not clear how to test both implementation branches -- for PHP >= 5.2 and PHP < 5.2.
Comment 3 Van de Bugger 2011-11-12 08:32:27 UTC
Created attachment 9428 [details]
Patch v3.

v2 + Default format is get from user preferences:

{{ #time: | 2011-11-10 23:00 }} 

will print specified date/time in user-preferred format. The same is about #timel.

Tests are not added, because it depends on user preferences.
Comment 4 Bawolff (Brian Wolff) 2011-11-17 00:19:19 UTC
I don't really like #time: with no formatting string being user pref. It should probably have some symbol to specify that user time preference is wanted.

Having timel be user preference time is also something I'm not sure how i feel about. If we want that it should perhaps be a different name then something already used for server time. (For example, we intentionally don't user user-time in sigs. Personally I don't like that we have user time preferences at all, I think it just adds confusion, but thats off topic and just my personal opinion) However, if we do make timel be user time, then we'd need to make sure user time preferences are added to parser cache key, or it will be all mixed up (same with if formatting for user time preference, but we already mostly have support for that given #dateformat).


However, these objections don't really apply to the original patch
Comment 5 badon 2011-11-17 00:37:20 UTC
The world would be a better place if everyone just used UTC time. I set my servers to UTC time, which simplifies things if I have to move them. So, having the ability to reformat it to local time is helpful until the world moves to standard UTC time (or stardates!).
Comment 6 Van de Bugger 2011-11-17 21:00:51 UTC
> I don't really like #time: with no formatting string being user pref. It 
> should probably have some symbol to specify that user time preference is
> wanted.

#time does not interpret format string, but pass it to Language::sprintfDate. I do not like an idea to parse format string in #time -- it is not trivial due to backslashes, double quotes, di- and trigraphs. So for #time simplicity, format string should be passed to Language::sprintfDate. The latter can be modified asily to recognize one more special character. But, since it is just a place holder for user-specific format string, it should be parsed. Natural way to parse format string is pass it to Language::sprintfDate… This makes Language::sprintfDate a recursive function, so we should take special care about infinite recursion. Let us don't overcomplicate this stuff. Empty format string is good enough for denoting user-defined date/time format.

> Having timel be user preference time is also something I'm not sure how i feel
> about. If we want that it should perhaps be a different name then something
> already used for server time.

Who (except sever admins) takes care of server time? Server can be located in any place in the world, in any timezone, it should not make difference for users.

> Personally I don't like that we have user time preferences at all, I think it
> just adds confusion, but thats off topic and just my personal opinion

For sites like Wikipedia, timestamps probably are not too important, and can be displayed in UTC or server time. But I am working on site based on Semantic MediaWiki, it is a kind of database, where timestamps are important -- it is one of basic datatypes. All the timestamps are stored in UTC, but they are presented to a user in user local time (with a help from #timel).

> However, if we do make timel be user time, then we'd need to make sure user
> time preferences are added to parser cache key, or it will be all mixed up

Which cache? Do you mean this case (a beginning of `time' function in `ParserFunctions_body.php'): 

    if ( isset( self::$mTimeCache[$format][$date][$language][$local] ) ) {
        return self::$mTimeCache[$format][$date][$language][$local];
    }

Err… I am not experienced Mw-PHP-Web hacker… Is this variable shared between different HTTP requests? If not, it seems everything is in place.

I am worrying about another cache. Can MW cache entire pages and return already rendered page to different users? If so, either result of #timel will be reported incorrectly, or #timel can invalidate cache and load the server heavily. However, I am not sure, it is just a guessing.
Comment 7 Bawolff (Brian Wolff) 2011-11-18 14:14:38 UTC
(In reply to comment #6)
> > I don't really like #time: with no formatting string being user pref. It 
> > should probably have some symbol to specify that user time preference is
> > wanted.
> 
> #time does not interpret format string, but pass it to Language::sprintfDate. I
> do not like an idea to parse format string in #time -- it is not trivial due to
> backslashes, double quotes, di- and trigraphs. So for #time simplicity, format
> string should be passed to Language::sprintfDate. The latter can be modified
> asily to recognize one more special character. But, since it is just a place
> holder for user-specific format string, it should be parsed. Natural way to
> parse format string is pass it to Language::sprintfDate… This makes
> Language::sprintfDate a recursive function, so we should take special care
> about infinite recursion. Let us don't overcomplicate this stuff. Empty format
> string is good enough for denoting user-defined date/time format.


That's a good point. It doesn't entirely seem right to me to have the empty string be the user pref, but I'll withdraw my objections on that point.

Although, one could also just combine #time and #dateformat to achieve the same affect.

> > Having timel be user preference time is also something I'm not sure how i feel
> > about. If we want that it should perhaps be a different name then something
> > already used for server time.
> 
> Who (except sever admins) takes care of server time? Server can be located in
> any place in the world, in any timezone, it should not make difference for
> users.

Often times you want to display the same info to everyone (for example in user signatures, the common argument is that if they were all user time, then when people copied and pasted the time to someone else, people would get confused what the actual time is, since different people would see different times. Server time often represents where the majority of the user base is located. For example Korean Wikimedia projects use the time zone Asia/Seoul as server's local time zone.


> 
> > However, if we do make timel be user time, then we'd need to make sure user
> > time preferences are added to parser cache key, or it will be all mixed up
> 
> Which cache? Do you mean this case (a beginning of `time' function in
> `ParserFunctions_body.php'): 
> 
>     if ( isset( self::$mTimeCache[$format][$date][$language][$local] ) ) {
>         return self::$mTimeCache[$format][$date][$language][$local];
>     }
> 
> Err… I am not experienced Mw-PHP-Web hacker… Is this variable shared between
> different HTTP requests? If not, it seems everything is in place.
> 
> I am worrying about another cache. Can MW cache entire pages and return already
> rendered page to different users? If so, either result of #timel will be
> reported incorrectly, or #timel can invalidate cache and load the server
> heavily. However, I am not sure, it is just a guessing.

Yes, I meant the cache that cache's the rendered html version of the page, and displays it to other users (we refer to it as the parser cache). We do store rendering options with the parser cache, which can allow the cache to vary by user preference based on user options. (For example, we vary based on user's preferred date format in the preferences). In order to make the #time's vary by user timezone preference, the timezone would have to be added to the list of things the parser cache varies by, which could potentially have some negative performance impact, since it would fragment the cache (although if it was a sufficient amount of an impact to care, is something I don't know).
Comment 8 Van de Bugger 2011-11-19 13:15:49 UTC
> …It doesn't entirely seem right to me to have the empty string be the user
> pref,…

Why? It is a normal practise -- if no argument is explicitly specified, some default is used. The only minor difference that it is not named but positional argument, so I cannot completely omit it, but leave it empty instead.

> Although, one could also just combine #time and #dateformat to achieve the 
> same affect.

Do not see how. #dateformat does not accept time and do not accept timezone. It works with dates only. So, it is possible to get formatted date:

    {{ #formatdate: {{ #time: Y-m-d | 2011-11-10 01:00 YAKT }} }}

But how to format time? I did not find a way.

> Often times you want to display the same info to everyone (for example in user
> signatures, the common argument is that if they were all user time, then when
> people copied and pasted the time to someone else, people would get confused
> what the actual time is, since different people would see different times.
> Server time often represents where the majority of the user base is located.
> For example Korean Wikimedia projects use the time zone Asia/Seoul as server's
> local time zone.

I believe #time is exact for such a case -- when you want to display the same info for everybody. BTW, nowadays, every timestamp should include timezone not to confuse what actual time is.

Korean Wikipedia is a good example… But I guess there are many Korean-speaking people in the USA. And there are countries which do not fit into one timezone. And I am from such a country, so I have to worry about users in multiple timezones.

However, I have a thought… If all the timestamps are in UTC, and properly formatted, for example: 

    <span class="time">2011-11-10T23:00:15UTC</span>

it would be easy to translate it to user timezone and format with user preferences on the client side with JQuery script. Do you know how to pass user preferences to the client?

Ok, let us  get back to #time and #timel. You do object against #timel to be a user local time. If I change it to be a local server time will it be enough to apply the patch?
Comment 9 Daniel Friesen 2011-11-19 23:03:54 UTC
(In reply to comment #8)
> However, I have a thought… If all the timestamps are in UTC, and properly
> formatted, for example: 
> 
>     <span class="time">2011-11-10T23:00:15UTC</span>
> 
> it would be easy to translate it to user timezone and format with user
> preferences on the client side with JQuery script. Do you know how to pass user
> preferences to the client?

Btw, html5's <time> element may be useful here.
Comment 10 Bawolff (Brian Wolff) 2011-11-20 01:11:11 UTC
>it would be easy to translate it to user timezone and format with user
>preferences on the client side with JQuery script. Do you know how to pass user
>preferences to the client?

We actually already do that, for all user preferences.

>Ok, let us  get back to #time and #timel. You do object against #timel to be a
>user local time. If I change it to be a local server time will it be enough to
>apply the patch?

That would take care of most of my concerns. The other concern I have is you should use $parser->getOptions()->getDateFormat() instead of $wgLang->dateFormat(), and should probably be using $parser->getFunctionLang() instead of $wgLang in most (all) of the places you use $wgLang (need to double check that though)
Comment 11 Van de Bugger 2011-11-21 18:59:37 UTC
Created attachment 9517 [details]
Patch v4.

Summary: 

* Just fix for the bug. (Empty format is NOT a user preference; #timel is server local time -- as it was before.)

* Parser tests include tests for #time, because testing #timel depends on server's timezone.

* Both branches (PHP < 5.2 and PHP >= 5.2) are manually tested.
Comment 12 Van de Bugger 2011-11-21 19:03:54 UTC
> Btw, html5's <time> element may be useful here.

Thanks, will look at this.

> > Do you know how to pass user preferences to the client?
> We actually already do that, for all user preferences.

Please point me a sample how to get user preference from JQuery code.

> The other concern I have is you should use $parser->getOptions()->getDateFormat()
> instead of $wgLang->dateFormat(),

I will open another bug (enhancement) for it. Let us fix and close this one.
Comment 13 Van de Bugger 2011-11-29 20:44:47 UTC
Ping.
Comment 14 Bawolff (Brian Wolff) 2011-12-07 23:15:10 UTC
Sorry for the delay.

Applied in r105459. I'm going to mark this as fixed. Do you want to open separate bugs for the other stuff discussed earlier?
Comment 15 Bawolff (Brian Wolff) 2011-12-09 18:12:27 UTC
I split off the user display time preference to bug 32923. If people are interested in the use user timezone pref for displaying possibly with js magic, that should also be split off to a separate bug (but see also bug 14286)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links