Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with Daylight Savings Times #5

Open
adampmoss opened this issue Mar 16, 2020 · 5 comments
Open

Bug with Daylight Savings Times #5

adampmoss opened this issue Mar 16, 2020 · 5 comments

Comments

@adampmoss
Copy link

There's 100% an issue when setting time rules that overlap daylight savings times.

I have a rule that shows content every other week between 11pm and 11pm on Thursdays every 14 days.

Screenshot 2020-03-16 at 16 51 42

When a daylight savings switchover kicks in on Sunday 29th March at 1am, the following Thursday will incorrectly show my content between the hours of 10pm and 11pm.

I added the following code to debug in rulesShowHTML():

foreach ( $active_periods as $period ) {
            if ( ( $period['start'] <= $right_now_t ) && ( $right_now_t <= $period['end'] ) ) {
                echo get_the_title($id)." active because current time is between ".date('jS F Y H:i:s', $period['start'])."  and ".date('jS F Y H:i:s', $period['end'])."<br /><br />";
                $rule_is_active = true;
                break;
            }
        }

This outputs this on the frontend:

menu1 active because current time is between 2nd April 2020 22:00:00 and 9th April 2020 22:00:00

I think the issue is because we're setting a timezone here:

date_default_timezone_set( $timezone );

When I comment this out the code works as expected. I'm not 100% sure why but I think we want to avoid setting a timezone when working with UNIX timestamps because they should be timezone independent as described here: https://stackoverflow.com/a/23062640/750537

The incorrect setting of the times then gets messed up here in the same getRulePeriods() function:

$instance_start = strtotime( $this->datetimeToEnglish( $instance_start_date,
                $instance_start_time ) . " " . $timezone );    // Beginning of first occurrence
$instance_end   = strtotime( $this->datetimeToEnglish( $instance_end_date,
                $instance_end_time ) . " " . $timezone );            // End of first occurrence

My brain was melting reading this code, I hope you can offer some insight here?

@arnowelzel
Copy link
Owner

Just some quick thoughts about this issue:

First of all: the times in rules or shortcodes are not unix timestamps but always local times. So the timezone must be used as well. So if we remove the timezone from the calculation at least the current time on the server has to be converted to the local time according to the timezone of the rule.

I think the problem is another: If the start date is before a DST switch, it may move to another time after the DST switch. A workaround might be to add or remove an hour depending on wether the DST of the start date is different to the current date or not.

@adampmoss
Copy link
Author

Is this a fix you could implement any time soon? I honestly don't feel like I understand this code or problem well enough to do it at this point.

@adampmoss
Copy link
Author

So dumping out the timezone on the frontend in the rulesShowHTML() file I can see that it's set as "UTC", whereas the timezone being used to calculate the time periods is is "Europe/London".

function rulesShowHTML( $atts, $content = null )
    {
        global $post;
        var_dump(date_default_timezone_get());

Screenshot 2020-03-17 at 10 00 02

I'm assuming if we can get the frontend to match the backend that will do the job.

@arnowelzel
Copy link
Owner

Sorry, but I'm really busy with other things right now. Don't expect a fix within the next 7-10 days.

@adampmoss
Copy link
Author

Understood, for the time being I'm going to have to add my fix and comment out line 826 date_default_timezone_set( $timezone );

I'll keep an eye on here for any updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants