Opened 6 years ago
Closed 5 years ago
#44985 closed enhancement (wontfix)
timezone_string should be valid when offset time zone mode is used
Reported by: | Rarst | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Date/Time | Keywords: | needs-patch needs-unit-tests |
Focuses: | Cc: |
Description
timezone_string
time zone settings mode is backwards compatible, overriding gmt_offset
mode with correct offset:
// Kiev
var_dump( get_option( 'gmt_offset' ), get_option( 'timezone_string' ) );
// float(3) string(11) "Europe/Kiev"
However the reverse isn't true, if settings are in offset mode the time zone string is empty and invalid:
// UTC+3
var_dump( get_option( 'gmt_offset' ), get_option( 'timezone_string' ) );
// string(1) "3" string(0) ""
In part this is because on older PHP versions there is no valid string that would correspond to an hours offset. But from PHP 5.5.10 it would work with timezone strings in +03:00
format just fine.
I think generating valid time zone strings in offset mode for new PHP versions is low hanging and makes things much more robust for downstream developers, making it harder for them to unknowingly screw up by relying on timezone_string
alone.
We still need unified timezone retrieval in #24730, but that doesn't displace making timezone_string
by itself more reliable.
Will work on a patch.
Change History (10)
This ticket was mentioned in Slack in #core-datetime by rarst. View the logs.
6 years ago
#3
in reply to:
↑ 2
;
follow-up:
↓ 4
@
6 years ago
Replying to JPry:
We are not doing any mapping for this one. It is impossible to reliably inflate the data from less meaningful to more meaningful.
I am merely talking about making timezone_string
return +NN:NN
format offsets on newer PHP versions, which can use that as valid input, just as any geographical timezone string.
#4
in reply to:
↑ 3
@
6 years ago
Replying to Rarst:
I am merely talking about making
timezone_string
return+NN:NN
format offsets on newer PHP versions, which can use that as valid input, just as any geographical timezone string.
Ah, I misunderstood your description. Looking forward to seeing the enhancement.
#6
@
6 years ago
I've implemented a naive implementation for this, but ran into some warnings that use the timezone_string and can't handle the +HH:MM format.
Notice: date_default_timezone_set(): Timezone ID '+02:00' is invalid in /srv/www/wordpress-trunk/public_html/build/wp-admin/options-general.php on line 264
My incomplete implementation was:
add_filter( 'option_timezone_string', 'wp_timezone_override_string' );
function wp_timezone_override_string($string_offset) { if(!empty($string_offset)){ return $string_offset; } remove_filter( 'pre_option_gmt_offset', 'wp_timezone_override_offset' ); // Remove filter to prevent infinite loop. if ( ! $gmt_offset = get_option( 'gmt_offset' ) ) { return $string_offset; } $offset = gmdate('H:i', floor(abs($gmt_offset) * HOUR_IN_SECONDS)); $prefix = '+'; if($gmt_offset < 0){ $prefix = '-'; } return $prefix . $offset; }
#7
@
6 years ago
I got conversion logic down with unit tests and everything in https://github.com/Rarst/wpdatetime/blob/master/src/WpDateTimeZone.php
Yeah, those IDs don't work with date_default_timezone_set()
but empty string case also doesn't work with it. And if anyone is calling date_default_timezone_set()
in WP context they are "doing it wrong". In core that case works because it explicitly checks for empty.
That said we should get rid of that date_default_timezone_set()
altogether there. It's one of those really really old pieces of Date/Time. I'll make a ticket for that.
#8
@
6 years ago
Oh and also after PHP 5.6 requirement bump we can just do this unconditionally, so I am for adding it to the waiting pile for that.
@Rarst
I've worked on this recently in a plugin with the idea of fully allowing for any GMT offset or timezone string. If it helps, there are a few challenges that you'll find with this:
timezone_name_from_abbr()
andtimezone_abbreviations_list()
functions has changed. You'll get different results for PHP < 7.0 and PHP >= 7.0.Hopefully that helps save you a few headaches. 🙂