Page MenuHomePhabricator

Local timestamps wrong during Daylight Saving Time (DST; Summer Time)
Closed, ResolvedPublicBUG REPORT

Description

Summary: As described here (archived here, and mentioned again here), while daylight saving time (summer time) is in effect, the times shown in NavPops are all one hour earlier than reality.

Details:
My time zone is set to "America/Los Angeles" here, and it currently says (correctly): "Server time: 11:35 Local time: 04:35" (the server is in UTC and local time here is UTC-7 (PDT) during DST).

When hovering over a "hist" link in a contribs or page history list (e.g. this), the time column shows times that are an hour early (02:59:40 in this example, for an edit that occurred at 10:59:40 UTC = 03:59:40 PDT).

When hovering over the "diff" link for that edit (here), it shows: "New Revision 2019-05-11 02:59:40", also an hour early.

When hovering over that page title at 11:43 UTC (04:43 PDT) (here), though, it correctly calculates: "43 minutes old".

Another example: hovering over this, shows:

New revision 2019-05-02 19:07:25
Old revision 2019-02-27 09:32:42

The New time (during DST) is wrong but the Old time (not DST) is correct – both are calculated as UTC-8.

Event Timeline

AlanM1 renamed this task from Time offset calculation incorrect after DST transition to Local timestamps wrong during Daylight Saving Time (DST; Summer Time).May 23 2019, 6:10 PM
AlanM1 triaged this task as High priority.
Tacsipacsi raised the priority of this task from High to Needs Triage.Sep 29 2019, 7:35 PM
Tacsipacsi subscribed.

Priority should be set by developers, not the reporter. See https://www.mediawiki.org/wiki/Phabricator/Project_management#Setting_task_priorities

My original report of this bug at enwiki has just passed its sixth birthday.

@AlanM1: I don't see how age of an issue is relevant when it comes to bringing a task forward? If the previous comment is 'just' about venting frustration, then also see https://www.mediawiki.org/wiki/Bug_management/Development_prioritization - thanks! :)

Based on the previous reports (neither logout-login nor cache clear helps, but toggling time zone setting in the preferences does), this may be a bug in how MediaWiki core handles time zone preference.

I believe I looked at the code at some point. WIth my limited knowledge of whatever language it was written in, I think I determined that the problem is that it is (incorrectly) determining the offset based on the current time and then applying that offset to all timestamps, regardless of when they are.

Instead, it needs to process each timestamp individually, determining the correct offset for that individual timestamp. In the example above, the "old" revision occurs during winter time (UTC-8 for me), while the "new" revision occurs during summer time (UTC-7 for me). It calculated the offset as -7 hours based on the current time (when I wrote that ticket) and then applied that offset to all timestamps, regardless of what time of year they occurred.

Note that the "right" solution is not to try to pre-calculate two offsets and transition datetimes either. Especially in Muslim countries, there can be multiple transitions during the year (e.g., during Ramadan), and governments globally are making many changes to their timekeeping rules, changing timezones, DST transition dates, etc. This is all carefully tracked by the tz database, which stores all those historical (and some future) transitions, and its API, which is called by the various time functions. The only safe/correct solution is to process each timestamp individually, using the functions made for exactly that purpose. I think there's a doc somewhere in the tz distribution that discusses these best practices and common "pitfalls".

@JJMC89 Are you sure the preferences work as intended? The symptoms I listed show they probably don’t.

@Tacsipacsi The preferences are storing the time zone correctly – it's the use of the time zone in adjusting the UTC timestamps displayed in NavPops that's the problem (see my comment above). I stand corrected (by JJMC89 below) – I didn't realize it was storing the offset, which doesn't make sense, since it's incorrect to just add that offset to any timestamp to get a local time.

Depending on when (DST vs not DST) a user sets the timezone preference, it could be stored differently, e.g. ZoneInfo|-360|America/Chicago or ZoneInfo|-300|America/Chicago (format: part 1: "System", "ZoneInfo", or "Offset"; part 2: offset in minutes; part 3: name [only for ZoneInfo]; with the parts separated by pipes).

In all cases, Navigation popups is using the offset in minutes, which may not match the timezone currently. For the ZoneInfo case, MediaWiki uses the name of the timezone.

https://en.wikipedia.org/wiki/MediaWiki:Gadget-popups.js

Navigation popups
function getTimeOffset() {
	var tz = mw.user.options.get('timecorrection');

	if(tz) {
		if( tz.indexOf('|') > -1 ) {
			// New format
			return parseInt(tz.split('|')[1],10);
		} else if ( tz.indexOf(':') > -1 ) {
			// Old format
			return( parseInt(tz,10)*60 + parseInt(tz.split(':')[1],10) );
		}
	}
	return 0;
}

https://github.com/wikimedia/mediawiki/blob/06e235b99fe2d9b69383198a25ce2c55a4eb73c4/includes/MWTimestamp.php#L71-L135

MediaWiki
	/**
	 * Adjust the timestamp depending on the given user's preferences.
	 *
	 * @since 1.22
	 *
	 * @param User $user User to take preferences from
	 * @return DateInterval Offset that was applied to the timestamp
	 */
	public function offsetForUser( User $user ) {
		global $wgLocalTZoffset;

		$option = $user->getOption( 'timecorrection' );
		$data = explode( '|', $option, 3 );

		// First handle the case of an actual timezone being specified.
		if ( $data[0] == 'ZoneInfo' ) {
			try {
				$tz = new DateTimeZone( $data[2] );
			} catch ( Exception $e ) {
				$tz = false;
			}

			if ( $tz ) {
				$this->timestamp->setTimezone( $tz );
				return new DateInterval( 'P0Y' );
			}

			$data[0] = 'Offset';
		}

		$diff = 0;
		// If $option is in fact a pipe-separated value, check the
		// first value.
		if ( $data[0] == 'System' ) {
			// First value is System, so use the system offset.
			if ( $wgLocalTZoffset !== null ) {
				$diff = $wgLocalTZoffset;
			}
		} elseif ( $data[0] == 'Offset' ) {
			// First value is Offset, so use the specified offset
			$diff = (int)$data[1];
		} else {
			// $option actually isn't a pipe separated value, but instead
			// a comma separated value. Isn't MediaWiki fun?
			$data = explode( ':', $option );
			if ( count( $data ) >= 2 ) {
				// Combination hours and minutes.
				$diff = abs( (int)$data[0] ) * 60 + (int)$data[1];
				if ( (int)$data[0] < 0 ) {
					$diff *= -1;
				}
			} else {
				// Just hours.
				$diff = (int)$data[0] * 60;
			}
		}

		$interval = new DateInterval( 'PT' . abs( $diff ) . 'M' );
		if ( $diff < 1 ) {
			$interval->invert = 1;
		}

		$this->timestamp->add( $interval );
		return $interval;
	}
TheDJ claimed this task.

This change was deployed and I think it should be fixed now.