Skip to content

[Form] Add time zone translations #17628

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

Closed
wants to merge 2 commits into from

Conversation

c960657
Copy link
Contributor

@c960657 c960657 commented Jan 31, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #13662
License MIT
Doc PR -

This patch adds translation files for time zone names based on Unicode CLDR data (see e.g. http://www.unicode.org/cldr/charts/27/by_type/timezones.north_america.html).

The translations are generated using a simple PHP script that must be run when there are changes to the CLDR data. The files must then be regenerated and committed to Git.

The generator only generates translations for locales already supported by the Form component (the CLDR contains a lot more).

The CLDR repository does not contain translations of the continent names. This must be done using the override files in translations-source. I have added the file for the da locale (Danish).

The patch is against the 2.7 branch. This is when the choice_translation_domain option was added.

A patch for 2.3 will be based on the assumption that the default messages domain is used for the form control. I will provide a separate PR for that.

@c960657 c960657 force-pushed the timezone-translation branch from 431978b to 68dbc8e Compare January 31, 2016 17:02
@c960657 c960657 changed the title Timezone translation [Form] Add time zone translations Jan 31, 2016
@c960657
Copy link
Contributor Author

c960657 commented Feb 1, 2016

On the master branch we should use the new TimezoneBundle in the Intl component, see PR #17636.

@Tobion
Copy link
Contributor

Tobion commented Feb 1, 2016

I'm not convinced to translate timezone identifiers. Do you have an example app where this is done this way? Drupal does not translate them either.

Also this does not actually translate the timezones but only a subset of regions and cities. So you are not translating Europe/Paris but only Paris. But the timezone is Europe/Paris.

@Tobion
Copy link
Contributor

Tobion commented Feb 1, 2016

Also this is definitely not a bug fix but a new feature.

@c960657
Copy link
Contributor Author

c960657 commented Feb 1, 2016

I'm not convinced to translate timezone identifiers. Do you have an example app where this is done this way?

Google Calendar.

Mediawiki, Twitter and Tumblr, on the other hand, shows the English names in all locales.

The addition of timezone translations to Unicode Common Locale Data Repository is a fairly new thing. That may be part of the explanation why using the translated city names is not more widespread.

Other than web apps, there is also the timezone selector in Microsoft Windows, the city names are shown in the system language.

Drupal does not translate them either.

They zones are translateable in Drupal (at least versions 7 and 8), though I have not found a locale that actually translates them.

Also this does not actually translate the timezones but only a subset of regions and cities. So you are not translating Europe/Paris but only Paris. But the timezone is Europe/Paris.

The TimezoneType currently only displays the city names (included in an optgroup with the continent name), not the entire identifier, e.g. the dropdown contains "New York", not "America/New_York". The patch does not change that.

In most locales, some city names are not translated, either because they do not have a name in the specific language (usually only large cities have that), or because they simply have not been translated yet. In the CLDR files, the name from the "en" locale is used as a general fallback. I think this is a reasonable choice given the circumstances.

I think that the city names in the TimezoneType control should be shown in the current language, just like the country names in the CountryType control. This is particularly relevant when timezone and current language match (i.e. when the language is spoken in the city that identifies that timezone), because most users will select such timezones (i.e. a lot of Italian-speaking users will use the Europe/Rome timezone).

For example, assume we have an Russian-language site that is visited by an Russian user living in Moscow, i.e. all text on the site is in Russian. In a CountryType control, the user will see the Russian-language name for Russia, Росси́я. As an end-user (who does not speak English and is not familiar with the Olson database), I would expect the TimezoneType control to contain Москва́, not Moscow. I would find it strange to see the English name for the Russian capital on a site where all other texts are in Russian.

@xabbuh xabbuh added the Form label Feb 3, 2016
@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Dec 6, 2016
@stof
Copy link
Member

stof commented Mar 31, 2017

I would consider it a new feature

@Tobion
Copy link
Contributor

Tobion commented Mar 31, 2017

Indeed

@Tobion Tobion added Feature and removed Bug labels Mar 31, 2017
@Tobion Tobion removed this from the 2.7 milestone Mar 31, 2017
@curry684
Copy link
Contributor

I'm far from convinced by the quality of the source data. I briefly reviewed the Dutch translations as it's my native language, and I found a lot of weird things, like "Unknown" being translated to "onbekende stad", which is "unknown city". Also, country names are indeed stripped despite being relevant most of the time:

 +     <trans-unit id="c0f15fd3fae3f15e2c7042d1622e0ed1" resname="Argentina - San Juan">
 +        <source>Argentina - San Juan</source>
 +        <target>San Juan</target>
 +      </trans-unit>

There are tons of cities called San Juan on this planet, but the most obvious one is the capital of Puerto Rico, not the much smaller and less known province capital this time zone is named after. In a different time zone than Puerto Rico too (1 hour later).

So 👎 it is.

@stof
Copy link
Member

stof commented Apr 3, 2017

@curry684 you could contribute to CLDR to improve their Dutch translations. This would benefit all projects relying on it.

@srl295
Copy link

srl295 commented Apr 7, 2017

@c960657 thanks for filing CldrBug:10175 … and everyone, thanks for symfony!

The CLDR repository does not contain translations of the continent names

Actually it does. It has translations for the UN M.49 numeric region codes alongside territories. So, ZA = South Africa, 002 = Africa, etc.

to @stof 's point, yes, @curry684 please contribute to CLDR if you can improve Dutch. The San Juan issue is being discussed at the CLDR bug above. The id Unknown is actually Unknown City because that is the context of use.

Hope this helps.

@fabpot
Copy link
Member

fabpot commented Jul 6, 2017

Given that there is doubt about the current quality of the data (still relatively new in CLDR) and the fact that you cannot opt-out, I fear that people will complain about that. As it's possible to do it as a bundle, it would be a good first step. I'm closing this one in the meantime.

fabpot added a commit that referenced this pull request Apr 21, 2019
This PR was squashed before being merged into the 4.3-dev branch (closes #28831).

Discussion
----------

[Intl] Add Timezones

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | replaces #26851, #17628, #17636
| License       | MIT
| Doc PR        | symfony/symfony-docs#11447

This PR compiles a new `TimezoneBundle` from ICU data, based on the following resources:

- https://github.com/unicode-org/icu/tree/master/icu4c/source/data/zone
- https://github.com/unicode-org/icu/blob/master/icu4c/source/data/misc/timezoneTypes.txt
- https://github.com/unicode-org/icu/blob/master/icu4c/source/data/misc/metaZones.txt

The goal is to provide a fixed set of timezones, with a localized human readable name.

For inspiration and to double check some data i used the timezone widget from google, e.g. when using Google Calendar.

I've only pushed some common compiled locales for review, the rest will follow once finished.

cc @jakzal

Commits
-------

4bea198 [Intl] Add Timezones
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants