Remove confusing legacy logic in date_i18n()

[jdgrimes]

In #37634 it was pointed out that date_i18n() contains the following line:

$datefunc = $gmt? 'gmdate' : 'date';

However, there is actually no difference between date() and gmdate() in WordPress, so that line can be removed. At the time that this logic was added, WordPress was experimenting with setting the default PHP time based on the site's timezone. But in the end it was decided to just always have the default PHP timezome be UTC/GMT. Which means that date() and time() always return UTC values, and thus using gmdate() is unnecessary. (I discovered this just the other day when I was researching a date/time-related bug in a plugin.) See #9588 and [12727].

Retaining this legacy logic is confusing, since it implies that there is some difference in WordPress between using date() and gmdate(), when there really is none. date() should just be used instead.

Note that although that particular line in the function is unnecessary, the $gmt parameter is still needed for these lines:

<?php
    if ( false === $i ) {
        if ( ! $gmt )
            $i = current_time( 'timestamp' );
        else
            $i = time();
        // we should not let date() interfere with our
        // specially computed timestamp
        $gmt = true;
    }

However, they could be simplified to just:

<?php
    if ( false === $i ) {
        $i = current_time( 'timestamp', $gmt );
    }

We should probably consider removing every reference to gmdate() in core in favor of just using date(), to avoid this confusion. But maybe that would ultimately be for a separate ticket.

[jdgrimes]

Remove legacy logic and always use date() in date_i18n()

[jdgrimes]

It looks like gmdate() is used quite a few places in core (~50), so if we want to address that, it should be in a separate ticket I guess.

[swissspidy]

37910.2.diff​ adds some initial tests

[swissspidy]

In 38804:

Date/Time: Remove some legacy logic in date_i18n().

Since there's no difference between using date() and gmdate() in WordPress, we can simply use the former in date_i18n() to reduce its complexity.

Adds tests.

Props jdgrimes for initial patch.
Fixes #37910.

[pento]

The unit tests for this have a race condition, and are failing intermittently, see #38381.

[swissspidy]

In 38871:

Date/Time: Fix unit tests after [38804].

The tests for date_i18n() need to use a delta for comparing timestamps.

Fixes #38381. See #37910.

[johnbillion]

Tests_Date_I18n::test_adjusts_format_based_on_timezone_string() broke when Europe moved out of DST last night.

The test was updated in [39012] and [39013], but this needs to be investigated to see what exactly broke, or whether the test is faulty.

[swissspidy]

Tests_Date_I18n::test_adjusts_format_based_on_timezone_string() was added to cover this part of `date_i18n()` which replaces date format strings based on the site's timezone. So it's not really about comparing the time, but about checking whether that replacement works as expected. Hence the unusual date format string.

The logic in date_i18n() is still valid, but the test is faulty. I need to replace it with something a bit more flexible.

[ocean90]

Replying to swissspidy:

The logic in date_i18n() is still valid, but the test is faulty. I need to replace it with something a bit more flexible.

What about using a timezone which has the same UTC offset all year like ​America/Regina?

[swissspidy]

Replying to ocean90:

Replying to swissspidy:

The logic in date_i18n() is still valid, but the test is faulty. I need to replace it with something a bit more flexible.

What about using a timezone which has the same UTC offset all year like ​America/Regina?

That… makes sense.

[swissspidy]

@ocean90 Thanks for the patch! You might not have known, but uploading an attachment doesn't trigger a notification :P

[ocean90]

Right, I was ​waiting for the build which took a bit longer and so I forget to comment. And here's a little script to show that America/Regina has the same offset: ​https://3v4l.org/L59eg

[swissspidy]

In 39167:

Date/Time: Improve date_i18n() timezone tests.

This uses a timezone without DST for the tests covering date_i18n().

Fixes #37910.

[Rarst]

I would argue that this should be normalized on gmdate() on the contrary. If any third party code changes PHP timezone (yeah, it shouldn't but in practice happens all the time), all of that will be waaaay off in context of other issues with date_i18n().

Also see #38771

[swissspidy]

@Rarst Yeah unfortunately the same goes other date functions as well, see #38940.