This issue is part of meta issue #1931088: [META] Fixing tests

Original issue reported by @mongolito404

Currently, the location module use language independent provinces names. For instance, the name for the Belgian province with code WBR will always be ""Brabant Wallon", regardless of the language set for the site or by the user. But this is its French name and it has a different name in English ("Walloon Brabant") and in Dutch ("Waals-Brabant").

A solution may be to cache province names by language in location_get_provinces and to return translated names in location_province_list_$country (see attached patch).

ToDo

  1. Reroll it against latest 7.x-3.x-dev and after commit - backport to 6.x
  2. If we want #518908-31: [META] Support province name translation in - we should use English names in source strings for right t() handling
  3. divide it for many small patches per source file and create for each its own follow-up issue with this issue as META
  4. connect to http://localize.drupal.org groups and ask groups members for review
  5. Test coverage
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pbuyle’s picture

Version: 6.x-3.0 » 6.x-3.1-rc1
YesCT’s picture

Issue tags: +location translation

tagging.

Summit’s picture

Subscribing, will this patch be getting in?
Thanks a lot in advance for considering.
Greetings, Martijn

john.money’s picture

+1 for patch concept

A Russian user is going to be looking for Москва, not Moscow (City). The original patch is missing

return $provinces[$langcode][$country];

in the 'location_province_list_'. $country function call.

clashar’s picture

subscribe

emmajane’s picture

I tried applying this patch for an equivalent (Canadian) problem. The first hunk was applied (no problem), then I adapted the location.ca.inc file to wrap province names in t(). Then I re-loaded the language.inc file via the UI. Once this is done, what's actually supposed to happen? I'm not sure if I'm dealing with a failed patch or with caching issues...but I'm not seeing new strings to translate?

emmajane’s picture

Aha! I think I needed to refresh my translations at admin/build/translate/refresh. I now see Provinces listed as taxonomy terms...is that what you were expecting?

mvc’s picture

Version: 6.x-3.1-rc1 » 6.x-3.1

I'm using this patch on 3.1 and it works well. I made similar changes to location.ca.inc for Canadian province names. I did have to manually truncate the table cache_location in the database to get my changes to show up, however.

My remaining problem is that when using the views sort "location: provinces" the order is always according to the English (untranslated) names. I'll attach a further patch for that when I can. I thought I could I write a handler for that which uses the province name instead of the abbreviation but in fact views sorts require a field that is stored in the database meaning that data from the supported/ directory would be tricky to handle. I think this would require another cache table someplace to allow writing an ORDER BY clause to catch this but that would be a major change in addition to a big mess so I'm not touching it until province and country names are stored in the db (if that ever happens). So it's an ugly template hack for me :(

mvc’s picture

Status: Active » Reviewed & tested by the community

forgot to change status

ShaneOnABike’s picture

This patch also works for me although I had to modify the location.ca.inc -- can we deploy this for all countries it would be pretty awesome!

Also, has anyone run into an issue with city names not being translated. I'm scratching my head trying to figure out how to resolve that.

joray.open’s picture

FileSize
1.28 KB

Error code

<?php

// China

function location_province_list_cn() {
  return array(
    '11' => "Beijing",
    '12' => "Tianjin",
    '13' => "Hebei",
    '14' => "Shanxi",
    '15' => "Nei Mongol",
    '21' => "Liaoning",
    '22' => "Jilin",
    '23' => "Heilongjiang",
    '31' => "Shanghai",
    '32' => "Jiangsu",
    '33' => "Zhejiang",
    '34' => "Anhui",
    '35' => "Fujian",
    '36' => "Jiangxi",
    '37' => "Shandong",
    '41' => "Henan",
    '42' => "Hubei",
    '43' => "Hunan",
    '44' => "Guangdong",
    '45' => "Guangxi",
    '46' => "Hainan",
    '51' => "Sichuan",
    '52' => "Guizhou",
    '53' => "Yunnan",
    '54' => "Xizang (Tibet)",
    '61' => "Shaanxi",
    '62' => "Gansu",
    '63' => "Qinghai",
    '64' => "Ningxia",
    '65' => "Xinjiang",
    '71' => "Taiwan",
    '91' => "Xianggang",
    '92' => "Aomen",
    '97' => "Chongqing",
    '98' => "Gaoxiong",
    '99' => "Taibei",
  );
}

/**
 * Returns minimum and maximum latitude and longitude needed to create a bounding box.
 */
function location_bounds_cn() {
  return array(
    'minlng' => 73.5403,
    'minlat' => 16.0968,
    'maxlng' => 134.7658,
    'maxlat' => 53.608867,
  );
}

The correct code

<?php

// China

function location_province_list_cn() {
  return array(
    'BEJ' => t("Beijing"),
    'TAJ' => t("Tianjin"),
    'HEB' => t("Hebei"),
    'SHX' => t("Shanxi"),
    'NMG' => t("Nei Mongol"),
    'LIA' => t("Liaoning"),
    'JIL' => t("Jilin"),
    'HLJ' => t("Heilongjiang"),
    'SHH' => t("Shanghai"),
    'JSU' => t("Jiangsu"),
    'ZHJ' => t("Zhejiang"),
    'ANH' => t("Anhui"),
    'FUJ' => t("Fujian"),
    'JXI' => t("Jiangxi"),
    'SHD' => t("Shandong"),
    'HEN' => t("Henan"),
    'HUB' => t("Hubei"),
    'HUN' => t("Hunan"),
    'GUD' => t("Guangdong"),
    'GXI' => t("Guangxi"),
    'HAI' => t("Hainan"),
    'SCH' => t("Sichuan"),
    'GUI' => t("Guizhou"),
    'YUN' => t("Yunnan"),
    'TIB' => t("Xizang (Tibet)"),
    'SHA' => t("Shaanxi"),
    'GAN' => t("Gansu"),
    'QIH' => t("Qinghai"),
    'NXA' => t("Ningxia"),
    'XIN' => t("Xinjiang"),
    'TAI' => t("Taiwan"),
    'HKG' => t("Xianggang"),
    'MAC' => t("Aomen"),
    'CHQ' => t("Chongqing"),
    'KHH' => t("Gaoxiong"),
    'TPE' => t("Taibei"),
  );
}

/**
 * Returns minimum and maximum latitude and longitude needed to create a bounding box.
 */
function location_bounds_cn() {
  return array(
    'minlng' => 73.5403,
    'minlat' => 16.0968,
    'maxlng' => 134.7658,
    'maxlat' => 53.608867,
  );
}

Please fix it!

Summit’s picture

Hi, Are things like this still be committed to Drupal 6 please?
Thanks a lot in advance for considering this!
Greetings, Martijn

yang_yi_cn’s picture

I think this is also an issue for 7.x, #284910: Location and Localization

podarok’s picture

Status: Reviewed & tested by the community » Postponed

postponed before tests fix
#1931088: [META] Fixing tests

podarok’s picture

Version: 6.x-3.1 » 6.x-3.x-dev
Status: Postponed » Needs review

tests fixed #1931088: [META] Fixing tests
lets go

Status: Needs review » Needs work

The last submitted patch, location.patch, failed testing.

jerdiggity’s picture

Is there a reason we can't add non-existent province names to the {locales_source} table? Patch to follow...

jerdiggity’s picture

Status: Needs work » Needs review
FileSize
3.65 KB

Patch alters the location_util_form form, giving the option to scan each location.xx.inc file for province names and insert those names, if they don't already exist, into the {locales_source} table.

jerdiggity’s picture

That patch (#18) still needs to add support for actually displaying the translated province names... Please do not commit.

jerdiggity’s picture

Re-submitting patch from #18, but with support for displaying province names depending on user's current locale. If it passes automated testing, please verify before committing (obviously)... ;)

Status: Needs review » Needs work

The last submitted patch, location-supportprovincenametranslation-518908-19.patch, failed testing.

jerdiggity’s picture

Grr... I'm tired (but I *was* on a roll there for a little bit). One more shot.

jerdiggity’s picture

Status: Needs work » Needs review
FileSize
5.64 KB

GRRRR....

Status: Needs review » Needs work

The last submitted patch, location-supportprovincenametranslation-518908-20.patch, failed testing.

podarok’s picture

+++ b/location.admin.incundefined
@@ -381,3 +394,55 @@ function location_util_form_clear_supported_countries_submit() {
+ * Location utilities form: Insert each province name from
+ * location_get_province_names_for_t() into to the {locale_sources} table ¶

trailing whitespace error
http://drupal.org/coding-standards#indenting

Tests failure related to #1926846-13: Extending gmap module *.tests. Catching test gaps.
Needs fix in follow-up before this commited

jerdiggity’s picture

Previously-submitted patch was not including array keys for each <option></option> on the location widget, so each option's value was 0, 1, 2, 3, etc., instead of the province code.

Resubmitting along with removed trailing whitespace as per #25. (Good catch!) Although I'm not getting my hopes up... ;)

jerdiggity’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests +location translation
FileSize
18.29 KB

@podarok: If you have a chance, could you verify (or let me know) if this is supposed to look like this? I'm referring to the option class... It happens on the location widget while using a select widget, and it only appears this way after the country has been changed and the provinces have been updated based on the selected country (see attached).

Status: Needs review » Needs work

The last submitted patch, location-supportprovincenametranslation-518908-25.patch, failed testing.

jerdiggity’s picture

Status: Needs work » Needs review
FileSize
334.17 KB

OK I'm now taking the desperate route... Any reason why wrapping each province name in t() wouldn't work?

jerdiggity’s picture

jerdiggity’s picture

Assuming this next patch will pass, it should address the following issues:

/me crosses fingers

podarok’s picture

Wow, huge patch

+++ b/supported/location.ad.incundefined
@@ -2,16 +2,6 @@
-    'SJL' => "Sant Julia de L�ria");
-}

@@ -23,3 +13,15 @@ function location_bounds_ad() {
+    "SJL" => t("Sant Julia de L�ria"),
+  );

Looks like we should replace these characters with something more readable

+++ b/supported/location.at.incundefined
@@ -27,3 +13,17 @@ function location_bounds_at() {
+    "NOS" => t("Niederösterreich"),
+    "OOS" => t("Oberösterreich"),

and this...

+++ b/supported/location.cf.incundefined
@@ -33,3 +13,25 @@ function location_bounds_cf() {
+    "MKD" => t("Mambere-Kade�"),
+    "MBO" => t("Mbomou"),

we should replace this character with something more readable

+++ b/supported/location.cm.incundefined
@@ -26,3 +13,18 @@ function location_bounds_cm() {
+    "EXN" => t("Extreme North Province (Extr�me-Nord)"),
+    "LIT" => t("Littoral Province"),

we should replace this character with something more readable

+++ b/supported/location.ee.incundefined
@@ -31,3 +13,23 @@ function location_bounds_ee() {
+    "49" => t("J�geva County"),
+    "51" => t("JArva County"),
+    "57" => t("LAAne County"),
+    "59" => t("LAAne-Viru County"),
+    "65" => t("P�lva County"),
+    "67" => t("PArnu County"),
+    "70" => t("Rapla County"),
+    "74" => t("Saare County"),
+    "78" => t("Tartu County"),
+    "82" => t("Valga County"),
+    "84" => t("Viljandi County"),
+    "86" => t("V�ru County"),

we should replace these characters with something more readable

+++ b/supported/location.fr.incundefined
@@ -68,3 +44,109 @@ function location_bounds_fr() {
+    "F35" => t("Ille-et-Vilaine - Bretagne"),
+    "F56" => t("Morbihan - Bretagne"),	

trailing whitespaces
http://drupal.org/coding-standards#indenting

+++ b/supported/location.fr.incundefined
@@ -68,3 +44,109 @@ function location_bounds_fr() {
+    "G45" => t("Loiret - Centre"),	
+    "H08" => t("Ardennes - Champagne-Ardenne"),

trailing whitespaces
http://drupal.org/coding-standards#indenting

+++ b/supported/location.kp.incundefined
@@ -31,3 +15,19 @@ function location_bounds_kp() {
+    "PYB" => t("P'yongan-bukto"),
+    "PYN" => t("P'yongan-namdo"),

Can we use ' in t() ? Needs chech or possibly test coverage

+++ b/supported/location.ru.incundefined
@@ -110,4 +16,97 @@ function location_bounds_ru() {
     'maxlng' => 180.000000,
     'maxlat' => 81.8583637,
   );
-}
\ No newline at end of file

http://drupal.org/coding-standards#indenting

+++ b/supported/location.wf.incundefined
@@ -22,4 +14,11 @@ function location_bounds_wf() {
     'maxlng' => -176.12456,
     'maxlat' => -13.209063,
   );
-}
\ No newline at end of file
+}
+function location_province_list_wf() {

http://drupal.org/coding-standards#indenting

+++ b/supported/location.ws.incundefined
@@ -30,4 +14,19 @@ function location_bounds_ws() {
     'maxlng' => -171.43666,
     'maxlat' => -13.462898,
   );
-}
\ No newline at end of file

http://drupal.org/coding-standards#indenting

This patch has a lot of possible internationalization issues, so my proposals are:

  1. Reroll it against latest 7.x-3.x-dev and after commit - backport to 6.x
  2. If we want #518908-31: [META] Support province name translation in - we should use English names in source strings for right t() handling
  3. divide it for many small patches per source file and create for each its own follow-up issue with this issue as META
  4. connect to http://localize.drupal.org groups and ask groups members for review
  5. Test coverage
podarok’s picture

Issue summary: View changes

Updated issue summary.

podarok’s picture

Title: Support province name translation » [META] Support province name translation
Version: 6.x-3.x-dev » 7.x-3.x-dev
Issue tags: -location translation

Status: Needs review » Needs work

The last submitted patch, location-supportprovincenametranslation-518908-30.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

podarok’s picture

Assigned: Unassigned » jerdiggity

updated summary du to #32

podarok’s picture

Issue tags: +Needs tests

+ tag

podarok’s picture

Issue summary: View changes

Updated issue summary.

podarok’s picture

Issue summary: View changes

Updated issue summary.

skyredwang’s picture

The last submitted patch, 31: location-supportprovincenametranslation-518908-30.patch, failed testing.

skyredwang’s picture

#31 patch is huge. and it doesn't apply to the latest head.

@podarok, you mentioned some non-english text. I would assume it's hard for one single person to correct all non-English texts for a few countries. How about we are leaving these texts as what they are, and rely on future requests to fix them?