WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9758 closed defect (bug) (fixed)

GMT-relative timezones inverted

Reported by: andy Owned by:
Milestone: 2.8 Priority: highest omg bbq
Severity: blocker Version:
Component: Date/Time Keywords: has-patch tested commit
Focuses: Cc:

Description

Set your timezone to GMT-5. The offset that is applied is +5.

Attachments (2)

9758.diff (1.9 KB) - added by Denis-de-Bernardy 5 years ago.
9758-untranslated.diff (1.4 KB) - added by Denis-de-Bernardy 5 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Denis-de-Bernardy5 years ago

  • Component changed from General to Date/Time
  • Keywords needs-patch added
  • Milestone changed from Unassigned to 2.8
  • Owner anonymous deleted

comment:2 MattyRob5 years ago

This appears to be affecting the timezone_string options when the TimeZone is set using Etc:GMT rather than location. It appears to be due to the wp_timezone_override_offset()

I have had a busy day and can't fully explain this but I think it is because we are comparing an offset time to GMT rather than a timezone to GMT so the offset is coming back in the correct magnitude but negative.

At the very end of this function the offset is converted into hours by dividing by 3600. If this is changed to a division by -3600 (note the negative) then it works for the Etc:GMT settings and break the other options!

comment:4 Otto425 years ago

This is not actually a bug, though it is confusing and we may need to address it.

PHP (and by extension, the new timezone stuff) uses the timezonedb/zoneinfo/Olson DB to get the list of zones. From the Olson DB comments:

We use POSIX-style signs in the Zone names and the output abbreviations, even though this is the opposite of what many people expect. POSIX has positive signs west of Greenwich, but many people expect positive signs east of Greenwich. For example, TZ=’Etc/GMT+4′ uses the abbreviation “GMT+4″ and corresponds to 4 hours behind UTC (i.e. west of Greenwich) even though many people would expect it to mean 4 hours ahead of UTC (i.e. east of Greenwich).

In other words, this is actually intentional.

We could add code to reverse the display of these in the dropdown listing to make it display more what people expect it to display, but internally, the representation is the way it needs to be for the zone correction mechanism to work correctly. Discuss, debate. The code is rather easy to do, just a simple regex to do the display name change while leaving the value blocks the same would do the trick.

comment:5 Otto425 years ago

Sorry, my quoting sucked. Here's what was in the Olson DB comments:

We use POSIX-style signs in the Zone names and the output abbreviations, even though this is the opposite of what many people expect. POSIX has positive signs west of Greenwich, but many people expect positive signs east of Greenwich. For example, TZ=’Etc/GMT+4′ uses the abbreviation “GMT+4″ and corresponds to 4 hours behind UTC (i.e. west of Greenwich) even though many people would expect it to mean 4 hours ahead of UTC (i.e. east of Greenwich).

comment:6 ryan5 years ago

I'm for reversing the display. Positive to the east seems to be the most common and accepted.

comment:7 MattyRob5 years ago

+1 for reversing it too. It's far too confusing to say that GMT+4 (so UTC PLUS 4 hours) is four hours BEHIND UTC!!

comment:8 Denis-de-Bernardy5 years ago

some pointers as to where things to be changed, so as to write the patch?

comment:9 Otto425 years ago

In wp-includes/functions.php, the wp_timezone_choice() function is what builds the dropdown list. This line of code is where you need to add something.

$structure .= "\t<option ".((($continent.'/'.$city)==$selectedzone)?'selected="selected"':'')." value=\"".($continent.'/'.$city)."\">$pad".str_replace('_',' ',$city)."</option>\n"; //Timezone

Basically, you need to change the display part of that (but not the value part) by inverting the +/- when the string matches GMT-X. Something like this.

$display = str_replace('_',' ',$city);

// regex code to match GMT-N and invert the sign goes here.

$structure .= "\t<option ".((($continent.'/'.$city)==$selectedzone)?'selected="selected"':'')." value=\"".($continent.'/'.$city)."\">$pad".$display."</option>\n"; //Timezone

Sorry I haven't had time to make up a patch myself, but I'm busy all week helping build our booth for the World Championship BBQ Festival. It's a massive affair. We're building an indoor beach. And naturally, BBQ comes first above all things. ;)

comment:10 Denis-de-Bernardy5 years ago

yeah, I should probably do that too, instead of spending 10-hours per day on trac for a full month...

comment:11 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested commit added; needs-patch removed

Denis-de-Bernardy5 years ago

comment:13 ryan5 years ago

$continents shouldn't be translated, should it? It's used only to whitelist zones, which are always in English, I think.

comment:14 ryan5 years ago

Speaking of translation, we might need to dump all contintent, cities, and subcities to a file and mark them for translation. We wouldn't need to include the file. It would just sit around to give xgettext something to parse.

comment:15 Denis-de-Bernardy5 years ago

Well, I don't mind reverting that, but continents are shown as colgroup labels. I can already hear an angry mob of horrible french (oh wait, I happen to be one :P) who will suggest that WP cannot translate continent names. ;-)

Untranslated patch coming right up. Pick the one you prefer.

comment:16 Denis-de-Bernardy5 years ago

while we're on the topic of translations, too, it may be worth asking outselves how to best translate the city names. Many have a 50s' colonial feel.

comment:17 Denis-de-Bernardy5 years ago

oh, hadn't read your second comment. :P

the first patch is the one to use imo.

comment:18 ryan5 years ago

Will the first patch even work though? Isn't it comparing an English name from zoneinfo with a translated name in this line:

if ( ! in_array($zone[0], $continents) )

comment:19 Denis-de-Bernardy5 years ago

aye. very true.

in that case the 2nd one is the correct one, and we'd want an extra file with both the continents and the cities/sub-cities as you suggested.

comment:20 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11288]) Fix sign of GMT offsets. Props Denis-de-Bernardy. fixes #9758

comment:21 ryan5 years ago

I'll work on the translations.

comment:22 Denis-de-Bernardy5 years ago

see #9794 for translations

Note: See TracTickets for help on using tickets.