alleyinteractive / wordpress-fieldmanager

Custom field types for WordPress
Other
532 stars 101 forks source link

Datepicker, store_local_time and daylight saving #759

Open petenelson opened 4 years ago

petenelson commented 4 years ago

We had an issue pop up recently when editing date/time in a custom field when the date and time happened after the 1hr change on daylight saving. The WordPress install is set to New York time, store_local_time is enabled. To replicate this, change the date/time of the event to anything after March 8, 2020 (the next time change), the hour gets reduced by one each time the post is saved (entered 2pm, saved it and it's 1pm, save it again and it's noon).

I was also able to replicate it when setting the date/time to prior to the recent 1hr time change (Nov 3rd). This seems to only happen when editing a time that falls outside the six-month window when there is no time change.

Some sample to drop into a dev site in order to replicate the issue.

<?php
/**
 * Plugin Name: Test Fieldmanager Time
 */

add_action( 'fm_post_post', 'test_fieldmanager_date_time' );

function test_fieldmanager_date_time() {

    $fields = [];

    $fields['start-time'] = new \Fieldmanager_Datepicker(
        [
            'label'            => __( 'Start Date/Time (Eastern)', 'stadium' ),
            'use_time'         => true,
            'store_local_time' => true,
            'default_value'    => current_time( 'timestamp', true ),
        ]
    );

    $fm = new \Fieldmanager_Group(
        [
            'name'           => 'event-settings',
            'serialize_data' => false,
            'children'       => $fields,
        ]
    );

    $fm->add_meta_box( 'Live Event Settings', [ 'post' ] );
}
mboynes commented 4 years ago

Thanks for the detailed issue report @petenelson! We'll dig into this bug and get a solution shipped.

betweenbrain commented 1 year ago

Hello,

We've also run into this issue, is there any chance that the fix for this could be considered for the next release?

betweenbrain commented 1 year ago

Hello again,

It appears this issue crept in for us with the release of v1.5.0.

mboynes commented 1 year ago

@betweenbrain thanks for poking at this. I had a solution ready and don't recall why it stalled out. Would you be able to test the PR #761 and confirm that it resolves your use case?

betweenbrain commented 1 year ago

@mboynes my pleasure. Unfortunately, I was not unable to reproduce the issue with a clean install of WordPress 6.1.1 and FieldManager 1.5.0. As soon as I can, I'll try applying PR #761 to a copy of our site with FieldManager 1.5.0.

Sean-TFG commented 1 year ago

Hello! I am able to confirm this issue is still happening in the latest version, found it on our site recently. To reproduce the error you would have to be testing on a day outside of DST, editing a date that exists inside DST. ( The opposite may work too. )

Under these circumcstances, when you update the post/item containing a datepicker using store_local_time, it will cause the value in the datepicker to be off by one hour, and if you hit update, this will then result in a drifting of one hour every time.

I was able to find the cause and come up with a solution listed below.

Within the datepicker class, you will find this code below in the form_element function.

if ( $this->store_local_time ) {
    $value += get_option( 'gmt_offset' ) * HOUR_IN_SECONDS;
}

It is pulling the offset as of the day you are rendering it, however it should instead pull the offset based on the date in $value. For example if I am editing this today, America/Los_Angeles would give me -8 for the offset, But if my datepicker was set to a time after the 12th, the correct offset would be -7.

In the presave function, it correctly accounts for this difference when get_gmt_from_date( $time_to_parse, 'U' ); is used. This is what causes the drift, an hour difference between the two functions, only when the above circumstances are met.

To fix this, I propose that the form_element function needs to correctly grab the offset based off $value ( the date ). Here I will give what code we are using to fix this.

if ( $this->store_local_time ) {
    //$value += get_option( 'gmt_offset' ) * HOUR_IN_SECONDS;

    $dt = (new DateTime())->setTimestamp($value);
    $offset = wp_timezone()->getOffset( $dt );
    $value += $offset;
}

Hope this helps.

mboynes commented 1 year ago

@Sean-TFG thanks for your note. We do already have a PR open for this -- can you confirm that #761 fixes the issue for you?

Sean-TFG commented 1 year ago

@Sean-TFG thanks for your note. We do already have a PR open for this -- can you confirm that #761 fixes the issue for you?

Yes I think that looks appropriate!