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 inlocation_province_list_$country
(see attached patch).
ToDo
- Reroll it against latest 7.x-3.x-dev and after commit - backport to 6.x
- If we want #518908-31: [META] Support province name translation in - we should use English names in source strings for right t() handling
- divide it for many small patches per source file and create for each its own follow-up issue with this issue as META
- connect to http://localize.drupal.org groups and ask groups members for review
- Test coverage
Comments
Comment #1
pbuyle CreditAttribution: pbuyle commentedComment #2
YesCT CreditAttribution: YesCT commentedtagging.
Comment #3
Summit CreditAttribution: Summit commentedSubscribing, will this patch be getting in?
Thanks a lot in advance for considering.
Greetings, Martijn
Comment #4
john.money CreditAttribution: john.money commented+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.
Comment #5
clashar CreditAttribution: clashar commentedsubscribe
Comment #6
emmajane CreditAttribution: emmajane commentedI 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?
Comment #7
emmajane CreditAttribution: emmajane commentedAha! 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?
Comment #8
mvcI'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 :(Comment #9
mvcforgot to change status
Comment #10
ShaneOnABike CreditAttribution: ShaneOnABike commentedThis 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.
Comment #11
joray.open CreditAttribution: joray.open commentedError code
The correct code
Please fix it!
Comment #12
Summit CreditAttribution: Summit commentedHi, Are things like this still be committed to Drupal 6 please?
Thanks a lot in advance for considering this!
Greetings, Martijn
Comment #13
yang_yi_cn CreditAttribution: yang_yi_cn commentedI think this is also an issue for 7.x, #284910: Location and Localization
Comment #14
podarokpostponed before tests fix
#1931088: [META] Fixing tests
Comment #15
podaroktests fixed #1931088: [META] Fixing tests
lets go
Comment #17
jerdiggity CreditAttribution: jerdiggity commentedIs there a reason we can't add non-existent province names to the
{locales_source}
table? Patch to follow...Comment #18
jerdiggity CreditAttribution: jerdiggity commentedPatch alters the
location_util_form
form, giving the option to scan eachlocation.xx.inc
file for province names and insert those names, if they don't already exist, into the{locales_source}
table.Comment #19
jerdiggity CreditAttribution: jerdiggity commentedThat patch (#18) still needs to add support for actually displaying the translated province names... Please do not commit.
Comment #20
jerdiggity CreditAttribution: jerdiggity commentedRe-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)... ;)
Comment #22
jerdiggity CreditAttribution: jerdiggity commentedGrr... I'm tired (but I *was* on a roll there for a little bit). One more shot.
Comment #23
jerdiggity CreditAttribution: jerdiggity commentedGRRRR....
Comment #25
podaroktrailing 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
Comment #26
jerdiggity CreditAttribution: jerdiggity commentedPreviously-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... ;)
Comment #27
jerdiggity CreditAttribution: jerdiggity commented@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).
Comment #29
jerdiggity CreditAttribution: jerdiggity commentedOK I'm now taking the desperate route... Any reason why wrapping each province name in
t()
wouldn't work?Comment #30
jerdiggity CreditAttribution: jerdiggity commentedOops... disregard. See next comment/patch (forgot to include #635958: French departments/provinces list not complete in location.fr.inc).
Comment #31
jerdiggity CreditAttribution: jerdiggity commentedAssuming this next patch will pass, it should address the following issues:
/me crosses fingers
Comment #32
podarokWow, huge patch
Looks like we should replace these characters with something more readable
and this...
we should replace this character with something more readable
we should replace this character with something more readable
we should replace these characters with something more readable
trailing whitespaces
http://drupal.org/coding-standards#indenting
trailing whitespaces
http://drupal.org/coding-standards#indenting
Can we use ' in t() ? Needs chech or possibly test coverage
http://drupal.org/coding-standards#indenting
http://drupal.org/coding-standards#indenting
http://drupal.org/coding-standards#indenting
This patch has a lot of possible internationalization issues, so my proposals are:
Comment #32.0
podarokUpdated issue summary.
Comment #33
podarok#31: location-supportprovincenametranslation-518908-30.patch queued for re-testing.
Comment #34.0
(not verified) CreditAttribution: commentedUpdated issue summary.
Comment #35
podarokupdated summary du to #32
Comment #36
podarok+ tag
Comment #36.0
podarokUpdated issue summary.
Comment #36.1
podarokUpdated issue summary.
Comment #37
skyredwang31: location-supportprovincenametranslation-518908-30.patch queued for re-testing.
Comment #39
skyredwang#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?