Opened 10 years ago
Closed 6 years ago
#29205 closed enhancement (wontfix)
Remove manual UTC offsets that are not officially used
Reported by: | nerrad | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.9.1 |
Component: | Date/Time | Keywords: | has-patch |
Focuses: | administration | Cc: |
Description
In doing some UTC offset to timezone string conversions for a project I came across some interesting anomalies in the possible options for UTC offsets.
There is the UTC+0:30 option... which is really interesting because apparently -> http://en.wikipedia.org/wiki/UTC%2B00:30
So I guess we need to keep back compat for all those folks who used WordPress back in 1936. ;)
Anyways, there are three other UTC offset options that don't appear to be in the official list of UTC offsets:
- 0:30
- 6:30
- 7:30
This definitely isn't a OMGBBQSAUCE type bug, but thought I'd bring it up nevertheless.
Attachments (2)
Change History (19)
#2
follow-up:
↓ 3
@
10 years ago
- Keywords reporter-feedback added
https://cloudup.com/cdPGi4BCGna
I see all of those on 4.0 and 3.9.2 - Where are you looking that you're not seeing this?
#3
in reply to:
↑ 2
;
follow-up:
↓ 4
@
10 years ago
Replying to Ipstenu:
I see all of those on 4.0 and 3.9.2 - Where are you looking that you're not seeing this?
I think the suggestion is to actually remove them from the "Manual Offsets" section of wp_timezone_choice()
.
#4
in reply to:
↑ 3
@
10 years ago
- Keywords reporter-feedback removed
Replying to SergeyBiryukov:
Replying to Ipstenu:
I see all of those on 4.0 and 3.9.2 - Where are you looking that you're not seeing this?
I think the suggestion is to actually remove them from the "Manual Offsets" section of
wp_timezone_choice()
.
Exactly, sorry it wasn't clear in my original post. The manual UTC offset list has some invalid UTC offset options. I'm suggesting that they be removed.
#5
@
10 years ago
For reference: http://en.wikipedia.org/wiki/List_of_UTC_time_offsets
At the bottom of that Wikipedia article you'll notice the UTC offsets they list as historical or unofficial in italics.
#7
@
10 years ago
- Summary changed from The "WordPress is really older than we think" UTC offset bug ticket. to Remove manual UTC offsets that are not officially used
- Type changed from defect (bug) to enhancement
- Version changed from trunk to 2.9.1
#8
@
10 years ago
It's *possible* these were added to address potential DST since when a UTC offset is selected automatic DST is not in play. I'll look into and will post back.
#9
@
10 years ago
How I discovered this, is I needed to convert any user's UTC offset selections to a close equivalent timezone string. To do this I grabbed the cities timezones info array from php's timezone_abbreviations_list()
and I looped through it, grabbing the 'offset' key and compared it with the selected UTC offset * 3600. I had some error control in place to detect if there wasn't a valid timezone string matching the given offset and that's where this surfaced.
I've attached a dump of all the offsets that are returned via timezone_abbreviations_list()
and as you'll see, there are no equivalent timezone strings found for -:30, +:30, -6:30, -7:30, and -8:30 (there may be others but those are the definite ones I've found).
As for:
I have no idea if somebody would actually be using one of these, but we'd need to know what would happen if they were and the option were removed.
I think to mitigate that, we'd likely have to have some sort of check to see if any invalid offset is saved in user db and then just bump it up to the closest offset matching it.
Really tho, I suspect this has gone unnoticed so long precisely because no one IS using these offsets, why would they if they don't actually exist in the real world :).
I'm willing to do a patch for this if desired... I just didn't want to do it until I knew it had any traction cause I've easily worked around it for my use case.
#10
@
10 years ago
another thought regarding how to handle users who have an invalid offset selected is to keep it in the db but when they visit the General Settings page, throw up a warning that their current offset is no longer valid and prompt to select a "correct" one or a timezone string. We could even preselect an offset closest to theirs (rounded up) in case they ignore the warning and just save.
#11
follow-up:
↓ 14
@
9 years ago
- Keywords has-patch added
I've gone ahead and created a patch that trims the manual offsets down to the valid set. One of the issues I found with people setting invalid manual offsets is that if you're doing some DateTime things in PHP and passed it an invalid offset to convert to timezone, it throws a fatal error. So I'm certainly in the camp of only providing a valid set of options.
To address Helen's concerns over "what if someone is using an invalid setting" ... nothing would change immediately. The setting persists in the database. The point it would change would be when someone goes to Options > General then saves something on that page. This would overwrite their current invalid timezone setting in the database. With an invalid option, the select will default to the first value so I added an additional check to reset this to 'UTC+0'. Otherwise it was selecting Africa/Abidjan which was likely far more undesirable as a replacement than UTC+0
#12
@
9 years ago
We should remove them all as they don't have time zone strings that are need in the code e.g. date_i18n()
#13
@
8 years ago
Added a patch in #36591 to hide them all if a timezone is selected. It would good to add this patch as well
#14
in reply to:
↑ 11
@
7 years ago
Replying to ryanduff:
I've gone ahead and created a patch that trims the manual offsets down to the valid set. One of the issues I found with people setting invalid manual offsets is that if you're doing some DateTime things in PHP and passed it an invalid offset to convert to timezone, it throws a fatal error. So I'm certainly in the camp of only providing a valid set of options.
Any more details on how you managed fatal? I was able to instance DateTimeZone from these "wrong" offsets just now without any issue.
More generally the main issue with numeric offsets in WP isn't that some of them are wrong, but that all of them are broken in how time zones are handled in localization. See #34835
#15
@
7 years ago
I'm guessing you instantiated DateTimeZone
with a UTC offset in a PHP 5.6+ box? Earlier versions of PHP do not provide that option. Also there's a PHP bug (https://events.codebasehq.com/redirect?https://bugs.php.net/bug.php?id=73489) which affects DateTime instantiated with a DateTimeZone
object using UTC offset.
#16
@
6 years ago
By itself I don't think these are worth messing with (and it doesn't look like we are dropping all offsets ever). However since we are trying to introduce unified time zone retrieval in #24730, if these explode that it would be very Not Good.
I am getting to writing unit tests for that, will see if these cause issues when it's done. If so I would probably be in favor of removing.
#17
@
6 years ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
I unit tested all offsets used by core for #24730 and there doesn't seem to be any technical issue.
Given that we don't really have visibility in how these might be used in the wild, I think messing with them is just asking for trouble and complicating the logic for no significant upside.
Also given that timezones are pretty impermanent and Murphy's law, I am guessing as soon as we remove them we would have to put them back in. :)
We can revisit if any technical problems surface and/or there is a change of circumstances with handling numeric offsets overall.
dang it... auto formatting messed up the UTC offset examples above. That should be:
and as mentioned already (tongue in cheek) +:30