Page MenuHomePhabricator

#time and #timel ignore explicitly specified timezone
Closed, ResolvedPublic

Description

Author: van.de.bugger

Description:
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

Version: unspecified
Severity: major

attachment ParserFunctions_body.php.patch ignored as obsolete

Details

Reference
bz32351

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:00 AM
bzimport added a project: ParserFunctions.
bzimport set Reference to bz32351.
bzimport added a subscriber: Unknown Object (MLST).

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

van.de.bugger wrote:

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.

  1. If timezone is not explicitly specified, UTC is assumed.
  1. #time output is UTC:

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

  1. #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).
  1. #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.
  1. 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:
  1. 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.
  1. It is not clear how to test both implementation branches -- for PHP >= 5.2 and PHP < 5.2.

attachment ParserFunctions-UTC.patch ignored as obsolete

van.de.bugger wrote:

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.

attachment ParserFunctions-UTC.patch ignored as obsolete

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

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

van.de.bugger wrote:

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.

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

van.de.bugger wrote:

…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?

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

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)

van.de.bugger wrote:

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.

Attached:

van.de.bugger wrote:

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.

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?

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)