WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 4 months ago

#49038 closed defect (bug) (fixed)

Timezone setting does not display correct time of next DST transition

Reported by: autotutorial Owned by: Rarst
Milestone: 5.3.3 Priority: normal
Severity: normal Version: 5.3
Component: Date/Time Keywords: has-patch fixed-major
Focuses: Cc:

Description

Currently wp_date when using a DateTimeZone other than UTC does not validate the timeline for daylight saving time for example for Europe/Rome summer time from 2020-03-29 02:00:00 to 2020-03-29 02:59 : 59, the timestamp is correct the problem is the generated output.
I have encountered this error from general settings, if I can hint when using date_create set a timezone and don't let php decide which timezone to apply.

Attachments (1)

49038-settings-dst-transition-time.patch (1.9 KB) - added by Rarst 5 months ago.

Download all attachments as: .zip

Change History (23)

#1 @mukesh27
5 months ago

  • Component changed from General to Date/Time

#2 follow-up: @Rarst
5 months ago

Please provide an example of timestamp that produces unexpected output, what result you expect for it, and what is actual result.

Last edited 5 months ago by Rarst (previous) (diff)

#3 in reply to: ↑ 2 @autotutorial
5 months ago

Replying to Rarst:

Please provide an example of timestamp that produces unexpected output, what result you expect for it, and what is actual result.

From the general settings set in Europe/Rome, it returns a date 2020-03-29 00:00:00 because the offset sum is incorrect and in the case of daylight saving time it must be returned in UTC 2020-03-29 02:00:00.
This means that in wp_date there is a bug and wordpress should only create dates (inputs) in UTC while the display (output) in any DateTimeZone except valid (DST from 2020-03-29 02:00:00 to 2020-03-29 02:59:59) which can only be displayed in UTC.
Only this is the correct way to use dates.
I hope it helps, I commented on the troubleshooting code in options-general.php.
On the way to creating the dates you need to think about, I am of the opinion that I only create UTC dates for all the date contexts and produce output (display) with the exception I mentioned earlier.
https://github.com/WordPress/WordPress/blob/5.3-branch/wp-admin/options-general.php#L283

<?php
$date_time_zone_selected = new DateTimeZone( $tzstring );
        //Bad 1 empty date create
                $tz_offset               = timezone_offset_get( $date_time_zone_selected, date_create( 'now', new DateTimeZone( 'UTC'  ) ) );
                $right_now               = time();
                foreach ( timezone_transitions_get( $date_time_zone_selected ) as $tr ) {
                        if ( $tr['ts'] > $right_now ) {
                //UTC with offset Daylight saving time
                                $found = true;
                                break;
                        }
                }

                if ( $found ) {
                        echo ' ';
                        $message = $tr['isdst'] ?
                                /* translators: %s: Date and time. */
                                __( 'Daylight saving time begins on: %s.' ) :
                                /* translators: %s: Date and time. */
                                __( 'Standard time begins on: %s.' );
                        // Add the difference between the current offset and the new offset to ts to get the correct transition time from date_i18n().
            //Bad sum offset, $tr['ts'] is UTC, offest with Daylight saving time is $tr['offset']
            //Here bad 2020-03-29 00:00:00 timezone Europe/Rome, it convert without sum $tz_offset - $tr['offset'], only sum 3600 ($tr['offset'] is 7200 with Daylight saving time) and convert UTC
            //UTC 2020-03-29 02:00:00
            //Europe/Rome wp_date bad here 2020-03-29 03:00:00
                        printf(
                                $message,
                                '<code>' . date_i18n(
                                        __( 'F j, Y' ) . ' ' . __( 'g:i a' ),
                                        $tr['ts']  + ( $tz_offset - $tr['offset'] )
                                ) . '</code>'
                        );
                } else {
                        _e( 'This timezone does not observe daylight saving time.' );
                }
        }
        ?>
Last edited 5 months ago by autotutorial (previous) (diff)

#4 @Rarst
5 months ago

  • Owner set to Rarst
  • Severity changed from critical to normal
  • Status changed from new to assigned
  • Summary changed from wp_date to Timezone setting does not display correct time of next DST transition

Think I got it.

The next transition in Europe/Rome time zone is at March 29, 2020 1:00 am UTC which is March 29, 2020 3:00 am in local time zone, but settings screen shows March 29, 2020 12:00 am due to date_i18n() output with incorrect offset math.

This might have regressed because date_i18n() previously didn't produce correct output across the DST boundary (because it added offset at the current time, not target time) and someone might have attempted to overcorrect by providing double-adjusted timestamp as input to it.

Basically that needs to be scrapped and replaced with sane wp_date() call, will do.

#5 @autotutorial
5 months ago

this is true before WordPress 5.3 existed.
I do not currently agree WordPress has always been wrong to create dates.
If the maximum portability of an independent date is desired from its offset (with daylight saving time applied or without the time applied or even with a different offset) .. WordPress creates dates only in UTC, the TimeZone only works on the output ( which must not correspond to daylight saving time) but the files created must be UTC otherwise it will not be compatible if I create a site in (UTC if the database used by the DateTime class is not updated produces different offsets depending on whether or not the offset is applied current.) and later set a timezone.
So from a file created in UTC it is always possible to apply its offset, take as an example the russia sometimes apply summer time other times no.
Since all the time zones are based on the UTC difference, creating UTC and displaying only that date in a timezone will be independent of the database used by the DateTime or offset class.
I'm sorry I don't want to devalue your work but the timezone is not valid for the whole world if it is used to modify a date, while if you have a UTC date you can apply the timezone for the whole world (ie extract the date in UTC and display it in Timezone without further changes).
I wrote it for completeness, I know that I did not express myself in the most correct manner.
In summary, the time must be expressed in UTC if it falls within the range of summer time.
March 29, 2020 2:00 am

Last edited 5 months ago by autotutorial (previous) (diff)

#6 @Rarst
5 months ago

We have considered moving to canonical UTC time, but so far decided against it since UTC storage is poorly suited for future dates. Which is necessary for scheduling functionality for example.

Storing future times in UTC in unreliable since they are meant to occur at specific local time by the user and the future state of time zones and corresponding UTC time is unknowable.

#7 @autotutorial
5 months ago

In the past it was so the DateTime class starts from the year 0000 to 9999 ... Is there something I did not understand, can you, please give me an example of a future UTC date with the DateTime class?
var_dump(new DateTime('9999-12-31 23:59:59.999999', new DateTimeZone('UTC')));

#8 @Rarst
5 months ago

  1. Imagine user schedules a post at 15:00 their local time.
  2. Let's say we convert it to 13:00 UTC and rely on that for everything.
  3. Meanwhile user's country decided to switch to a different time zone.
  4. 13:00 UTC no longer results in 15:00 local time when post is to be published.

From user perspective this is broken, they scheduled on one time and it got posted on another.

From application perspective it's impossible to predict what UTC time is going to correspond with future local time, because future state of time zones is unknowable.

Last edited 5 months ago by Rarst (previous) (diff)

#9 @autotutorial
5 months ago

Edit:

<?php
$tz_offset               = timezone_offset_get( $date_time_zone_selected, date_create( 'now', $date_time_zone_selected ) );
                $right_now               = time();
                foreach ( timezone_transitions_get( $date_time_zone_selected ) as $tr ) {
                        if ( $tr['ts'] > $right_now ) {
                                $found = true;
                                break;
                        }
                }

                if ( $found ) {
                        echo ' ';
                        $message = $tr['isdst'] ?
                                /* translators: %s: Date and time. */
                                __( 'Daylight saving time begins on: %s.' ) :
                                /* translators: %s: Date and time. */
                                __( 'Standard time begins on: %s.' );
                        // Add the difference between the current offset and the new offset to ts to get the correct transition time from date_i18n().
                        printf(
                                $message,
                                '<code>' . date_i18n(
                                        __( 'F j, Y' ) . ' ' . __( 'g:i a' ),
                                        $tr['ts'] + ( $tr['offset'] >= $tz_offset && $tz_offset > 0 ? $tr['offset'] - $tz_offset : ( $tz_offset < $tr['offset'] ? $tr['offset'] - $tz_offset : $tz_offset - $tr['offset'] ) )

thanks for the explanation if it is in a timezone it turns into UTC and will be displayed in timezone only as output (obviously if it is a convertible date).
I created a code improvement I can't create the patch, can you create it?
calculating the current offset in the timezone selected in general settings if they are in daylight savings time the variations have the same value, I make a subtraction and give the result which can be 0.

Last edited 5 months ago by autotutorial (previous) (diff)

#10 @Rarst
5 months ago

The offsets need to be dropped from here altogether. I worked it out already, will review and upload patch tomorrow.

#11 @autotutorial
5 months ago

Ok thanks a lot, I entered the ternary operator for 0 or less than zero.

#12 @Rarst
5 months ago

  • Keywords has-patch added
  • Milestone Awaiting Review deleted

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


5 months ago

#14 @ocean90
5 months ago

  • Milestone set to 5.3.3
  • Version set to 5.3

#15 @autotutorial
5 months ago

The bug has changed from the DateTime component to the logic structured in options-general.php.
the syntax UTC, Etc/GMT or any other synonym that has an offset 0 is considered a manual offset and in this case a manual offset regardless of its value does not have a Daylight saving time (old list timezone https://www.php.net/manual/en/timezones.others.php ).
I converted the page to a WordPress 5.3 syntax, where thanks to the creator of wp_timezone it was possible to apply other improvements.
In this example I removed the redundant calls to timezone_string and stored the value in a container variable since the value is the one previously recovered from general settings it is useful to keep the value for later use if the context remains to the WordPress core, more specifically it works only in that page context and will not be used for other pages.
I added wp_date with the timezone object created by wp_timezone or UTC, or UTC arithmetic operation, set UTC if $tzstring has an offset equal to 0, I added two unset to the wp_timezone_choice function ( use timezone_identifiers_list ), if possible I would like to avoid timezone_identifiers_list() if the timezone is in the list you should retrieve the value set from the menu, added restore array in $allowed_zones to free up php memory

<?php
if ( empty( $tzstring ) ) { // Create a UTC+- zone if no timezone string exists
        $check_zone_info = false;
        if ( 0 == $current_offset ) {
                $tzstring = 'UTC+0';
        } elseif ( $current_offset < 0 ) {
                $tzstring = 'UTC' . $current_offset;
        } else {
                $tzstring = 'UTC+' . $current_offset;
        }
} elseif ( 'UTC' == $tzstring ) {
    $check_zone_info = false;
}

?>
<th scope="row"><label for="timezone_string"><?php _e( 'Timezone' ); ?></label></th>
<td>

<select id="timezone_string" name="timezone_string" aria-describedby="timezone-description">
        <?php echo wp_timezone_choice( $tzstring, get_user_locale() ); ?>
</select>

<p class="description" id="timezone-description">
<?php
        printf(
                /* translators: %s: UTC abbreviation */
                __( 'Choose either a city in the same timezone as you or a %s (Coordinated Universal Time) time offset.' ),
                '<abbr>UTC</abbr>'
        );
        ?>
</p>

<p class="timezone-info">
        <span id="utc-time">
        <?php
                printf(
                        /* translators: %s: UTC time. */
                        __( 'Universal time is %s.' ),
                        '<code>' . wp_date( $timezone_format, null, new DateTimeZone( 'UTC' ) ) . '</code>'
                );
                ?>
        </span>
<?php if ( $current_offset ) : ?>
        <span id="local-time">
        <?php
        $timezone = wp_timezone();
                printf(
                        /* translators: %s: Local time. */
                        __( 'Local time is %s.' ),
                        '<code>' . wp_date( $timezone_format, null, $timezone ) . '</code>'
                );
        ?>
        </span>
</p>
<?php endif; ?>

<?php if ( $check_zone_info && $tz_string ) : ?>
<p class="timezone-info">
<span>
        <?php
        $now = new DateTime( 'now', $timezone );
        $dst = (bool) $now->format( 'I' );

        if ( $dst ) {
                _e( 'This timezone is currently in daylight saving time.' );
        } else {
                _e( 'This timezone is currently in standard time.' );
        }
        ?>
        <br />
        <?php
    //timezone_identifiers_list Avoid if possible
        $allowed_zones = timezone_identifiers_list();
        if ( in_array( $tzstring, $allowed_zones ) ) {
                                $transitions = $timezone->getTransitions( time(), time() + YEAR_IN_SECONDS );

                                if ( ! empty( $transitions[1] ) ) {
                echo ' ';
                            // Create timestamp UTC +- ( ex. 2+-1=1 or 2++1=3 ) for display Daylight saving time or Standard time.
                            $utc = $transitions[1]['ts'] + $transitions[0]['offset'];
                                $message = $transitions[1]['isdst'] ?
                                /* translators: %s: Date and time. */
                                __( 'Daylight saving time begins on: %s.' ) :
                                /* translators: %s: Date and time. */
                                __( 'Standard time begins on: %s.' );
                        printf(
                                $message,
                                                                '<code>' . wp_date( __( 'F j, Y' ) . ' ' . __( 'g:i a' ), $utc, new DateTimeZone( 'UTC' ) ) . '</code>'
                        );
                } elseif ( $current_offset ) {
                        _e( 'This timezone does not observe daylight saving time.' );
                }
        }
//You now, not have 467 array in memory
$allowed_zones = array( $tzstring );

You need to create a list for UTC synonyms versions and avoid getting a valid offset on 0, otherwise it is ambiguous, the code requires further changes

Last edited 5 months ago by autotutorial (previous) (diff)

#16 @Rarst
5 months ago

The bug has changed from the DateTime component to the logic structured in options-general.php.
the syntax UTC, Etc/GMT or any other synonym that has an offset 0 is considered a manual offset and in this case a manual offset regardless of its value does not have a Daylight saving time

I don't follow what do you consider a bug at this point. Please talk in specific examples of settings state, actual output, and output you think should be correct instead.

Etc/GMT (and other deprecated zones) are not supported by core and cannot be selected in settings.

Adding things to Unix timestamps - no, we are done doing that and wp_date() is written to work with real timestamps as is. I don't follow what output are you trying to achieve there.

Memory thing doesn't seem relevant, this is one-off page execution and it hardly matters if we try to release small amount of memory early.

Last edited 5 months ago by Rarst (previous) (diff)

#17 @autotutorial
5 months ago

Do not choose UTC+0 but UTC this time zone cannot be evaluated for daylight saving time or Standard time. (Only Universal Cordinated Time).
Europe/Rome Daylight saving time, Output UTC 2020-03-29 02:00:00
Europe/Rome Standard time, OUTPUT UTC 2020-10-25 03:00:00.
This is the only case of using arithmetic operations otherwise it creates confusion because it can be thought to be referred to before the change of Dayligth saving time or Standard time. also in the past they wanted to calculate the difference from local time (Dayligth saving time or Standard time) in arithmetic operation (I did the right arithmetic operation)
If I don't create a container variable $allowed_zones it is not possible to free 1500 KB.
wp_timezone_choiche + 1500 KB callback wp_timezone_choice + 1500 KB options-general.php + 1500 KB.
I am for the opinion of releasing superfluous 5 megabytes but I understand other points of view.

I edited the previous message, missing a function for the validity of a timezone string that uses an offset to 0.

Last edited 5 months ago by autotutorial (previous) (diff)

#18 @Rarst
5 months ago

With the patch I suggested the current output for Europe/Rome is Daylight saving time begins on: March 29, 2020 3:00 am which would be in CEST time zone.

I don't follow what is confusing about it or in what way would UTC be preferable.

This is a settings screen, not a clock. Personally I wouldn't even bother outputting time at all and only used date for this purpose here, but since it's in place already we are keeping it for consistency.

#19 @autotutorial
5 months ago

If you allow I express my idea of local time, however if it is a design choice I have nothing against it. The choice is yours.
forget any Dayligth saving time or Standard time in context for output.
In the past they tried to calculate the difference (wrongly), now if you always use an explicit timezone in DateTime all the data is safe.
If you calculate the timestamp offset (always UTC) and do the arithmetic operation you can create with UTC timezone and display in the correct way (even in the past it was like that, but with your help wp_date and UTC defined there is no margin of error).
Have you seen the bug if you select UTC?

Last edited 5 months ago by autotutorial (previous) (diff)

#20 @SergeyBiryukov
4 months ago

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

In 47073:

Date/Time: Use wp_date() to display the correct time of the next DST transition in Timezone setting on General Settings screen.

Props Rarst, autotutorial.
Fixes #49038.

#21 @SergeyBiryukov
4 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 5.3.3 consideration.

#22 @SergeyBiryukov
4 months ago

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

In 47127:

Date/Time: Use wp_date() to display the correct time of the next DST transition in Timezone setting on General Settings screen.

Props Rarst, autotutorial.
Merges [47073] to the 5.3 branch.
Fixes #49038.

Note: See TracTickets for help on using tickets.