WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 13 months ago

Last modified 9 months ago

#34835 closed defect (bug) (fixed)

date_i18n() produces invalid output for numerical time zones

Reported by: Rarst Owned by: flixos90
Milestone: 5.1 Priority: normal
Severity: normal Version: 3.1
Component: Date/Time Keywords: has-patch
Focuses: Cc:

Description

date_i18n() only attempts to use get_option( 'timezone_string' ) for timezone information. This option is empty when numerical offset is saved as time zone in settings.

Code to reproduce:

// Kiev
var_dump( get_option( 'timezone_string' ), get_option( 'gmt_offset' ) ); // 'Europe/Kiev', 2
var_dump( date_i18n( DATE_W3C, get_the_date( 'U' ) ) ); // 2015-12-03T12:22:22+02:00 correct

// UTC+2
var_dump( get_option( 'timezone_string' ), get_option( 'gmt_offset' ) ); // '', '2'
var_dump( date_i18n( DATE_W3C, get_the_date( 'U' ) ) ); // 2015-12-03T12:22:22+00:00 wrong

Attachments (4)

date_i18n.php (1.5 KB) - added by pbearne 3 years ago.
start of unit test
Added_numeric_time_zone_offset_support_to_date_i18n.patch (1.4 KB) - added by Rarst 16 months ago.
Implements correct output for numeric offsets, without going through time zone object.
Added_numeric_time_zone_offset_support_to_date_i18n1.patch (1.5 KB) - added by Rarst 16 months ago.
Skipped I format for DST which is irrelevant for the case.
trac-34835.patch (2.8 KB) - added by Rarst 13 months ago.
Patch with unit test.

Download all attachments as: .zip

Change History (15)

#2 @swissspidy
4 years ago

  • Version changed from trunk to 3.1

Introduced in [16203], see #8153

#3 @pbearne
3 years ago

The code tests if the options is returned and skips the code that uses it if not set

#4 @swissspidy
3 years ago

  • Keywords needs-patch added

@pbearne
3 years ago

start of unit test

#5 @pbearne
3 years ago

in the code, we test for if the option is empty/false and ship the use if it is

<?php
$timezone_string = get_option( 'timezone_string' );
if ( $timezone_string ) {....

So it doesn't error but it doesn't return a date with the offset

the problem is that you can have a more than one time zone for more than one offset and the summer/winter time changes are per timezone no offset.

There is a mapping from time zone to offset but not from offset to time zone as don't know which time zone to set

Maybe the need to discourage the setting of the offset in the options and that way always have a timezone.

The default enter for get_option( 'timezone_string' ) is emtpy not even GMT

#6 @Rarst
3 years ago

While the reliable conversion from offset to timezone string isn't possible it should still use offset in date output. Otherwise it's just clearly broken value, off from real value by that many hours.

Obviously only using real timezones would be preferable, but I am not sure if it's desirable/possible to drop offsets from UI (backwards compat and everything).

@Rarst
16 months ago

Implements correct output for numeric offsets, without going through time zone object.

#7 @Rarst
16 months ago

  • Keywords has-patch added; needs-patch removed

@Rarst
16 months ago

Skipped I format for DST which is irrelevant for the case.

@Rarst
13 months ago

Patch with unit test.

#8 @flixos90
13 months ago

  • Milestone changed from Awaiting Review to 5.0
  • Owner set to flixos90
  • Status changed from new to reviewing

#9 @flixos90
13 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 43387:

Date/Time: Add support for gmt_offset to date_i18n().

Prior to this change, date_i18n() only supported the timezone_string option, causing incorrect timezones to appear in formatted dates on sites that still rely on the gmt_offset option.

Props Rarst.
Fixes #34835.

#10 @flixos90
9 months ago

  • Milestone changed from 5.0 to 5.1

This ticket was mentioned in Slack in #core-datetime by rarst. View the logs.


9 months ago

Note: See TracTickets for help on using tickets.