Closed (fixed)
Project:
Location
Version:
6.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
2 Feb 2007 at 11:53 UTC
Updated:
22 Oct 2012 at 21:54 UTC
Jump to comment: Most recent file
Comments
Comment #1
csc4 commentedAnyone got any ideas please as this is causing big problems.
Comment #2
geodaniel commentedYou'd need to override the address display in location.uk.inc. I've a feeling that there is a patch floating around somewhere in the queue that improves upon support for the UK... If not though, you could take the German one (
function theme_location_de($location = array()) {), for example, put it into the UK file and rename it from _de to _uk. You may want to move the fields around to look more like a British address too.Comment #3
csc4 commentedThanks once again geodaniel!
Any idea if the patch is going to get committed as we UK's exist too!
Comment #4
geodaniel commentedI wonder if actually it should be a default for the location module to display the state/province name instead of code though. Most countries don't use those codes at all (Canada and America being the only two that spring to mind that do)...
Comment #5
csc4 commentedI've been thinking the same thing - and, in fact, I think this might actually be a bug.
If you use the shipped location_views location directory and drill down through country you get Cornwall (though there's a regrettable confusion between counties and 'areas' and poor old Wales and Scotland are a problem (but that's another issue!).
Digging in the code I noticed that the key seems to be line 898
Which seems to do a reverse lookup from the cryptic code to the 'pretty name' - which seems right.
I think the 'pretty name' should really be the one in $location... but I tried looking in the location module and I can't work out where it should be?
There's a test on line 787
Is that it?
Comment #6
csc4 commentedCan anyone offer some help on this please?
I'm pretty sure that it is a bug and that it should be the 'pretty name' displayed not the abbreviation but could do with some help pinning it down - and even better getting it fixed!
Comment #7
ankur commentedYes, the '$province_list_function' in this case is the function that pulls the array that matches a countries abbreviations for its provinces w/ the provinces' full names. In the case of the United States, this would be location_province_list_us() defined in modules/location/supported/location.us.inc . In the case of the UK (for which we'd be using counties), this would be location_province_list_uk().
In my opinion, the best way to do this would be to do it at the theming level, since it is the theming function (theme_location()) that outputs a location.
The way the theming functions works is that theme('location', $location), which, via the theming system, results in call to theme_location($location) defined in location.inc. This function then looks at the country in the location and checks to see, using the abbreviation for the country, whether a country-specific theming function exists for that country.
So, for example, if theme('location', $location) were called with $location['country'] == 'uk', it would first try to check if there was a function called theme_location_uk() and call that function instead. If there wasn't such a function, it would then continue through the default function, theme_location(), and use its code to theme the location, which, most likely, follows the US conventions for writing out an address since the author of the module is from the US.
In anycase, I think perhaps the best way to do this is to change the country functions to maintain a static array of their provinces and change them to take an optional 'abbreviation' argument. If the optional argument isn't supplied, it returns the entire array. If the optional abbreviation is given, then it returns the full name (which is looked up from the array where the province abbrevs are the array keys and the full names are the key values). So, for example, location_province_list_us() would still return array('AK' => 'Alaska', 'AL' => 'Alabama',....) while location_province_list_us('AK') would simply return the string 'Alaska'.
Of course, I'd prefer use of this function to determine full names to take place at the level of the country-specific theming function rather than at the default level theming function, because for some countries the abbreviation is good enough and saves CPU cycles, screen space, and memory space, albeit a neglible amount.
Comment #8
adminfor@inforo.com.ar commentedDuplicate closed http://drupal.org/node/129948
What about adding some parm in module, by country or global, or for displaying both taking defaults to work as usual.. thinking this is no posible due to code is in each _xx.inc file --- right?
Comment #9
geodaniel commentedAnkur, I hear what you're saying about extra resources being needed to load the name by default instead of the code but it seems to me that the default behaviour should be to display the name, as that is what is most countries use. For countries like the US and Canada that do typically only use the code (I wish we all did that, it'd make life simpler!) that could be overridden through the country-specific include to only use that instead of generating the full list of names, etc.
(Germany can be added to the list of countries that doesn't use the code.)
Comment #10
catchLooks still valid, reclassifying.
Comment #11
catchOK this can be fixed at the theme level and is a duplicate of http://drupal.org/node/134881
Comment #12
catchThis is the original issue, and it's not just an issue at the theme layer.
I'm trying out location 5.x-3.x-dev and the UK province autocomplete works pretty good - when you type in Essex, it finds "Essex", but when focus is taken away from the form, this gets changed to ESS - no-one ever uses or understands these codes in the UK. So ideally the form would display what's typed (since that's obviously present in the db) rather than the code it maps to.
Comment #13
catchNote that at least in location_views - the actual display is fine.
edit: here's a summary of behaviour I've found so far:
Autocomplete: - finds "Essex" but displays "ESS".
Node view - displays "ESS"
location_views - displays "Essex"
Search (which taken from node view) - finds "ESS" but not "Essex"
Province filter - finds "ESS" but not "Essex"
So it's fixed in some places, but not in others.
Comment #14
catchNot very elegant patch which takes the logic in location_views_province_handler and copies it to the relevant part of theme_location. Only tested with UK locations so far. Probably be better to make a helper function which does all this rather than duplicating the logic in multiple places. Marking to needs review though since it 'works'.
Comment #15
bdragon commentedI'm thinking that I should probabaly have the default be names, or at least have some sort of attribute that determines whether the code or name are used on a per-country basis.
On a side note, http://www.columbia.edu/kermit/postal.html is a great read.
Comment #16
summit commentedHi,
I am testing Location 3 test 2 (drupal 5).
I thing this same issue is at hand, but I do not see the lines in location_theme to alter.
Thanks for going into this!
greetings,
Martijn
Comment #17
Chad_Dupuis commentedI don't want to hijack this thread, but the code below may be helpful for folks working to get the full names out. Below is how I've been drudging out the full names of the country and province in the header section of a location views directory (with the latest 5.x-dev version, although it should be the same for 6). Not sure if there is an easier way of getting any of this info - the province in particular. I'm concerned about the resource load of doing this as it appears to be done already when first building the view. Anyhow, it meets the needs I have for now. Any feedback on a better way of accomplishing this would be appreciated.
Comment #18
summit commentedSubscribing
Comment #19
RickyS commentedI had the same issue and fixed it by adding following line in location_save function in location.module
$location['province_name'] = location_province_name($location['country'], $location['province']);
and changed location['province'] to location['province_name'] when it writes to database.
The code is located around lines 1078
Before:
After:
I tried it on version 5.x-3.1-rc1 and it's working.
Please confirm.
Comment #20
tryitonce commentedI am struggling with changing the looks / layout of location module as well as the abbreviated "province" names in the UK and elsewhere.
In the UK nobody would understand that NET refers to Newcastle on Tyne - etc. same applies Germany.
I am new to the "Location" Module - so - please be patient. I think it is great - but it needs some reality for abbreviating geographical names as well as which ones to use.
In the UK nobody would understand that NET refers to Newcastle on Tyne - etc. same in Germany - HB is Bremen - but mostly Germans would know that from the number plates. BB for Brandenburg or BE for Berlin just makes little sense.
For USA abbreviating geographical names might be common place - but not in most other countries. And then there is the question of which name should be used - take Germany again:
Niedersachsen that is clear for people in good knowledge of German & Germany - but internationally (English) the name is "Lower Saxony"; Thüringen would be Thuringia and thus lose the problematic "ü".
Would it not be better if developers could enter their own lists in a simple way - similar to CCK lists for text fields? - rather than hacking all those "location.countrycode.inc" files?
And finally just to get going for now - how can I apply the change in 6.x to display the full "province_name" instead of the abbreviated "province" code?
Unfortunately the suggestions above are for 5.x and I am not experienced enough to apply this straight to 6.x where the code looks different.
Thanks - and let me know if I can help with at least the German list
....
Comment #21
shenzhuxi commentedIn 6.x-3.1-rc1, Just change the 19 line of location.tpl.php into:
$city_province_postal[] = ''. $province_name .'';
Comment #22
cimbeles commentedThank you shenzhuxi !
It worked perfect in my webpage
Comment #23
deensdale commentedThank you shenzhuxi! I also made the following change in location.module, around line 639, to display the province name in the form field.
Comment #24
yesct commentedtagging
location.uk.inc
location.xx.inc
also there is a semi related discussion about the UK in #326665: UK cities are in state/province field
this issue is old... if the newer solutions in comments #21 and #23 are the way it can be fixed now, please make a patch for them:
http://drupal.org/patch/create
Comment #25
hutch commentedHere are patches for comments #21 and #23
tested on latest dev, both work just fine. Tested using uk and us as country
Comment #26
yesct commentedreleated to #771710: State Field should not allow input of abbreviations (any user input that is not an actual province data item) breaks gmap
mentioned in #2 in a nice summary and explanation: http://drupal.org/node/771710#comment-2849116
which explains that the codes should be internal, not used on input (related to #771710: State Field should not allow input of abbreviations (any user input that is not an actual province data item) breaks gmap) or during output (this issue)
there is also an issue about views sorting on the state/province code instead of the name... thinking of a new tag to group these related issues
Comment #27
yesct commentedoh, maybe I was thinking of this one about country names vs codes #380842: Views sort order incorrect for Country field
other thought. maybe the codes should be internal by default, but I could see some people wanting the option of using the codes (either during input or output), so maybe we need a configuration option (or view handler??? alternative).
Also, this is the dev, and I think most people using location are aware that a bunch of things are changing... but in an ideal dream world would we make an upgrade path that sets current installs to have the 'use code' configuration checked and new installs have it unchecked by default? Too much work, right? We have bigger fish to fry (We have more important fixes to work on).
Comment #28
roball commentedI tried to edit the
location.tpl.phptemplate (residing in my default theme directory) by changing$provinceto$province_name, which resulted in printing nothing instead of the province code in the node (using Location CCK). Did anyone test that using the Location CCK module (and not the classical Node Locations)?Comment #29
roball commentedSorry - just found out that I had the "Province name" checkbox ticked in the "Hide fields from display" settings of the Location field (admin/content/node-type/[NODE_TYPE]/fields/[FIELD_NAME]). After unticking it,
$province_namedisplayed properly :-)I have tested both patches from #25 with the latest 6.x dev and can confirm they are working fine :-) Thanks!
Comment #30
yesct commentedroball, thanks! We need people to test the patches. You are such a big help!
Comment #31
SeanA commentedThe patches in #25 also work with the latest 5.x dev.
The default behavior should be to display the full name, but maybe "use abbreviation" should be an option.
Comment #32
yesct commenteddo you think we need the "use abbreviation" config in order to get this committed?
I think if the default is to use full name, since that is different that what happens now, a special changelog/commit message would be needed to warn people that if they want to keep the code/abbreviation, they'll need to go in and change the config checkbox, right?
Comment #33
summit commentedHi Cathy, I agree with your latest remark, so people who are already working with it have the opportunity to change.
greetings, Martijn
Comment #34
hutch commentedCreating a checkbox under admin/settings/location is simple enough, it's applying that setting that has a few wrinkles.
Let's presume we already have the checkbox and this has created a new entry in the variable table which can be accessed wth
variable_get('location_use_abbreviation', 1)This BTW leaves the default of 'use abbreviation' intact.On the input side a change will have to be made in function location_locationapi circa line 639 in file location.module. No problem there:
'#default_value' => $obj,becomes'#default_value' => (variable_get('location_use_abbreviation', 1) ? $obj : location_province_name($country, $obj)),On the output side which is of course in location.tpl.php it gets a little trickier. I think this can be done in function template_preprocess_location() where an additional variable called say 'province_print' would contain the right output according to the setting stored in 'location_use_abbreviation'. This would give themers the choice and not change existing location.tpl.php instances which might well be in a theme folder.
The existing location.tpl.php would have
$province_printin place of$provinceHope this all makes sense.
Comment #35
SeanA commentedOK, using those ideas, here's a patch. This displays the full name by default, which fixes the wonky output of province codes in countries where they don't make sense. And there's an option that allows using the state codes if you want to.
Seems to be working. Should work on the 5.x branch too.
Comment #36
yesct commentedI'm not sure but
type syntax should probably be written as an if?
Comment #37
hutch commentedPersonally I don't have a problem with (condition ? 'do something' : 'do something else') syntax, use it all the time.
Comment #38
SeanA commentedShould it be written as an 'if' statement? I don't think so. The comment above the line in question explains what is happening.
Comment #39
hutch commentedI have re-rolled the patch on current CVS and tested it, works fine for me.
This patch will work with patch -p0
As far as I am concerned this patch is RTBC.
Comment #40
yesct commentedComment #41
rooby commentedThanks for everyone's work and testing.
I changed the name of the variable slightly so it's more descriptive and changed the default value so that the default is the current functionality.
Just so peoples sites don't change when they update the module.
Committed to D5, D6 & D7.
- http://drupal.org/cvs?commit=372148
- http://drupal.org/cvs?commit=372146
- http://drupal.org/cvs?commit=372144
Comment #42
roball commentedI have applied the patch from #39 to the latest dev. Works fine for me.
Comment #43
roball commentedResetting the status that has been changed due to crossing over my and the previous post.
I am now running the latest 6.x-3.x-dev (2010-May-28) which works perfectly fine for that issue. Just have to set "Province display" at admin/settings/location to "Display full province name." and it works as it should be.
Thank you for committing one of the most important fixes for the last year.
Comment #45
summit commentedHi, sorry for adding to closen issue. This is not working for me with -autocomplete- but it is working with province dropdown.
Just to let anyone know on which it is still also not working with autocomplete to have this option.
greetings, Martijn