Street 'Additional' field using only display setings for 'Street location' [#356058]

I have created a content type, enabled a single location, and configured the collection settings so the location fields are displayed on the node edit form in the following order:
Location name (weight: 1)
Street location (weight: 2)
Additional (weight: 3)
City (weight: 4)
State/Province (weight: 5)
Postal Code (weight: 6)
Country (weight: 7)

The problem is that on the node edit form, the 'Additional' field is always displayed before the 'Street location' field. I even went as far as setting the 'Street location' weight to -100 and the 'Additional' field weight to +100, but still the 'Additional' field was displayed directly before the 'Street location field'. I went into the code and noticed this (near line 210, location.module):

// Only include 'Street Additional' if 'Street' is 'allowed' or 'required'
if ($field == 'street' && $fsettings[$field]['collect']) {
  $element['additional'] = location_invoke_locationapi($defaults['additional'], 'field_expand', 'additional', 1, $defaults);
  $element['additional']['#weight'] = (int)$fsettings[$field]['weight'];
}

It appears that this code is overriding the custom 'Additional' field settings I set in the collection settings. It sets the weight of the 'Additional' field to the same as the 'Street location' field, and since 'Additional' comes before 'Street location' alphabetically, it is always displayed before 'Street location'. My guess is that this is some old code that was needed before it was possible to manually control the 'Additional' field's display. Now that we can, this code should probably be removed.

Comment	File	Size	Author
#9	location_additional_d6.patch	12.41 KB	jenlampton
#7	location_additional.patch	756 bytes	jenlampton
#1	location-356058-1.patch	835 bytes	pathfinderelite

Comments

Comment #1

pathfinderelite commented 8 January 2009 at 14:52

Status:

Active

» Needs review

Status	File	Size
new	location-356058-1.patch	835 bytes

Here is a patch that simply removes the unnecessary section of code.

Comment #2

vegagitator commented 8 January 2009 at 17:44

Deleting that code worked for me. Yay! Thank you.

Comment #3

j0rd commented 26 January 2009 at 12:05

Changing this in location.module . My code is for DRUPAL-6--3-0.

// Only include 'Street Additional' if 'Street' is 'allowed' or 'required'
if ($field == 'street' && $fsettings[$field]['collect']) {

to this...

// Only include 'Street Additional' if 'Street' is 'allowed' or 'required'
if ($field == 'street' && $fsettings[$field]['collect'] && $fsettings['additional']['collect']) {

..resolves the issue. I believe the author of this code only wanted to display "additional" if street is set to display and additional is set to display. If Additional is set to display and street isn't, he didn't want Additional to show up. Additional is like Address 2, which I've never understood anyways.

PS. Poor bug report name. Title the stuff better next time, so people like me can find the bugs easier.

Comment #4

pathfinderelite commented 26 January 2009 at 13:44

Title:

Street 'Additional' field is too smart for its own good

» Street 'Additional' field improperly displayed above 'Street' field

You make a good point about what the author wanted. However, this isn't a problem concerning when the additional field is displayed; its about where. The 'Additional' form field needs to be displayed below the 'Street' field on edit forms. I'm sorry if the bug report name was confusing.

Comment #5

vesapalmu commented 30 January 2009 at 13:10

I had the same problem, issue was fixed with j0rd's fix. Thanks.

Comment #6

j0rd commented 2 February 2009 at 21:22

No problem. It would be nice if we could get this fix into the next release. I wonder what a dude needs to do, to get some CVS dudes in here.

Comment #7

jenlampton

she / her

commented 7 February 2009 at 03:49

Title:

Street 'Additional' field improperly displayed above 'Street' field

» Street 'Additional' field using only display setings for 'Street location'

Status	File	Size
new	location_additional.patch	756 bytes

I see two different issues here...

The 'Additional' field IS or IS NOT displayed based on the setings for 'Street location'
The 'Additional' field always uses WEIGHT of 'Street location' field

The fix above solves the problem of Additional being added based on only the settings for Street location, but the Additional field still inherits the weight of the Street location field instead of retaining it's own. If the two weights are equal, Additional will come first (alphabetically). Updating the line that sets Additional's weight to that of street solved this other problem for me too.

Original code...

      // Only include 'Street Additional' if 'Street' is 'allowed' or 'required'
      if ($field == 'street' && $fsettings[$field]['collect']) {
        $element['additional'] = location_invoke_locationapi($defaults['additional'], 'field_expand', 'additional', 1, $defaults);
        $element['additional']['#weight'] = (int)$fsettings[$field]['weight'];
      }

My new code...

      // Only include 'Street Additional' if 'Street' is 'allowed' or 'required'
      if ($field == 'street' && $fsettings[$field]['collect'] && $fsettings['additional']['collect']) {
        $element['additional'] = location_invoke_locationapi($defaults['additional'], 'field_expand', 'additional', 1, $defaults);
        $element['additional']['#weight'] = (int)$fsettings['additional']['weight'];
      }

patch attached.