Returning "An illegal choice has been detected. Please contact the site administrator" when changing country

Thanks for the patch. I knew about this problem, but didn't have any idea what was causing it.

Unfortunately, when I follow your link, it takes me to the Drupal.org home page. Could you try re-attaching it? Drupal.org was just updated to D6, so that could be affecting things.

Finally, do you know how Ubercart avoids this problem? For instance, the addresses on the checkout form come from core Ubercart code and don't produce the "illegal choice" error. Does Ubercart do the same cache-update trick?

I can now see the patch, I think I understand most of what it does and what it does seems to make sense.

Please note that the patch is removing some important fixes that I recently added to uc_addresses_form_uc_cart_checkout_form_alter().

Well this patch adds the uc_addresses.js file on checkout so when the user selects a different country, our patched js file gets executed instead and thus applies the same retrieval and saving of the cached form too.

The uc_addresses system should not be fixing the checkout page (not everyone uses uc_addresses). However, if I disable uc_addresses, I can't reproduce this problem on the checkout page. Using grep, I couldn't find any use of form_set_cache() in Ubercart.

I may adopt this patch, but I would prefer to first understand how the order system avoids the problem and to use a similar approach so as to match Ubercart as closely as possible. I need to spend a little more time verifying the behavior of the system with and without uc_addresses. Meanwhile, the patch is a useful workaround for anyone struggling with this problem.

I appreciate your contribution. Thanks!

When I first tried your patch, it seemed to work. Then it didn't. I had made some other changes to uc_addresses, as well as some tweaks to the patch, so I wasn't sure where the problem was.

After a lot of debugging, I finally figured out that your patch gets the wrong build id if there are other forms on the page. So I changed

$('input[@name=form_build_id]').val()

to

$('form[@id^=uc-addresses] input[@name=form_build_id]').val()

Also, after a lot of searching through Ubercart code, I still have no idea how they avoid the problem.

I have not checked in the code yet because it still needs some more testing.

I've finally checked in my changes and they are available in the 6.x-1.x-dev release.

This fixes the problem when adding an address during registration or in the profile or when editing an address in the profile.

The problem remains when editing an address during checkout. A work-around is to not pre-fill in the user's default address. Ubercart uses some strange way of avoiding the "Illegal choice" error (it does not try to update the cached form) and this method fails if I fill in the fields.

The Ubercart developers don't seem to know how the checkout form avoids the error. I have not been able to figure out the method either, so this bug will be around for a while. Any help appreciated.

Can anyone provide some steps to replicate this problem.
The only error I get when changing the first time the country is a js error not visible if I don't use firebug. That error is there because I use jQuery 1.3 and @id and @name must be changed to just id and name in the js file. But aside that minor error I see no other error.

If I can replicate the problem maybe I can find a fix for it.

Hmmm.... I was hoping maybe the bug had disappeared due to changes in Ubercart and Drupal, but it is still there.

To duplicate:

1. Make sure you have some addresses defined and that the delivery and billing address are automatically filled in with the default address
2. Add a product to your cart and proceed to checkout
3. Change the delivery address's country
4. Select a zone
5. Select Review Order
6. I get the message: An illegal choice has been detected. Please contact the site administrator.

Thanks for taking this on. I have finally had a little bit of free time and have been trying to catch up on the uc_addresses issues, so I may finally get around to installing your other patches.

Can you try this patch and see if it solves the problem?

The problem is that adding on the fly options in a select box violates drupal security. The first time the form is build somehow it knows all the possible zones, but it only displays those for the current country. How it does that I didn't investigate (a solution would be to create an actual text field and replace that by javascript with a select box). But that wasn't my purpose. The fact is that the form_alter actually rebuild those select boxes, but without all the posible options.
What I did is to add the default values to what we know except the zone. For the zone I let that unchanged and only if the selected country is different from the store default country I force a javascript change of the zones (like you do when you actually change a country). That seems to fix the problem.
Additional changes are to simplify the code. It did 3 times a foreach on $addresses when the second and third time we only need the default address. I stored the default address on the first loop and used it when we need it. I also used the same if to set the default values so we don't check twice for almost the same conditions just to set different values. I also made the address selection select box show the default address if the address is displayed.

Congratulations!

The problem disappeared from the checkout form in my tests. Also, the changes didn't seem to affect the Add an Edit Address forms in the profile. They used to work and continue to work.

Things I didn't check:

• Address required during registration. Should work pretty much like the Add and Edit forms.
• Addresses not pre-filled on the checkout form. This should skip all the new code, which only executes if an address is filled in. Before this patch, that used to work and I assume it would still work since there should be no change.

Thanks for volunteering to tackle this problem. The code is checked in. I think it's time for a beta release.

I only changed the checkout alter function so it only affects the checkout form. Everything else works as before. If it worked before it will work now.

Actually, the Add and Edit Addresses functions in the user's profile depend on the same uc_addresses.js file that you altered and they used to depend on the same AHAH callback code that you removed, so the changes you made could have affected Add/Edit. However, Add/Edit seemed to work OK, so I'm not worried—and I'm certainly not complaining.

Now that I've mentioned the dependencies, if you decide to look at those two functions (they are in the uc_addresses_address_pane.inc file) just to make sure there isn't anything there that is being done incorrectly with respect to the zone stuff, I would appreciate that. Not a requirement, though. You've done more than your share.

If there are any problems, I'm sure someone will let us know—that's the benefit of crowdsourcing.

uc_address.js didn't do nothing.
It looked for a select box with an id ending in "country-" and there is none. What it was intendent to do was what uc_country_select.js already does.
That's why I removed the function it was suposed to call and the menu item calling it.
And if you check it out you will see that the same alter function I mentioned earlier is the only one that adds the js file.

It looked for a select box with an id ending in "country-" and there is none.

Technically, it was looking for a select box containing "-country-" (not necessarily ending with "-country-").

At some point, the id used to be there. I was unable to use uc_country_select.js because my id's were somehow coming out as "*-country-1" and the uc_country_select.js code wasn't able to handle it. If I recall properly, the Javascript was only intended for the Add/Edit address forms as they were the only ones with the problem.

For checkout, the ids were set without the added "-1" and I wanted to just use the Javascript that was already in Ubercart. But as soon as I filled in the country field (and I tried various ways of altering the form to do this), the error would show up. Before you made your changes, the uc_addresses.js was loaded for checkout, but that may have been one of my experiments. The idea was not to use it.

So now uc_addresses.js looks for a select box ending with "-country", However, Ubercart also has some javascript code looking for the same thing. Using firebug, when I load the checkout form and look at country select, I see:

<select id="edit-panes-delivery-delivery-country" class="form-select required ucCountrySelect-processed ucAddressCountrySelect-processed" name="panes[delivery][delivery_country]">

so the field was processed twice. If I comment out the use of uc_addresses.js for checkout, I get:

<select id="edit-panes-delivery-delivery-country" class="form-select required ucCountrySelect-processed" name="panes[delivery][delivery_country]">

However, the zone is not set. It may be that the double-processing is required to get the zone to work.

On the Add/Edit front, firebug tells me that the select is processed just once—and only by the uc_country_select.js! I think this is because of the test for Drupal.settings.uc_address_default that you added to uc_addresses.js. In any case, with the "-1" gone, they now work with uc_country_select.js.

To summarize:

• Somewhere along the way, id's of country-1 switched back to country, which is what they should have been all along. (I was not just writing crazy code! :-)
• The change essentially turned off the uc_addresses.js code
• The Add/Edit forms work fine using just uc_country_select.js. I can remove the two calls to load uc_addresses.js.
• The checkout form should be processed just once, but is getting processed twice. However, disabling uc_addresses.js causes the zone field to remain "Please select". Country changes work fine.

So the checkout form is still not being handled optimally, but the code seems to work (for now). If you can think of some reason the zone is not being set properly, we should be able to complete delete uc_addresses.js.

Before, even if the uc_address.js had something to select was doing the same thing as the country_select.js. The function it called was not fullfiling it's purpose so uc_address.js was practically useless.
But now it's not. I use it exactly to set the corect zone. Unless you don't want the correct zone selected don't remove that js file. It processed twice because the field is processed by uc_addresses which triggers a country change whic in turn triggers uc_country_select.js that changes the zones and selects the correct zone.

Ok, thanks. I worked through the code. I don't understand the details of all the AHAH/Form magic, but I'm starting to understand your changes.

When the Drupal.behaviors.ucAddressesCountrySelect behavior gets called, it finds the select fields and immediately calls uc_update_zone_select() with the default address (which you set up only when a default address is needed).

uc_update_zone() does its magic. It does NOT generate a change event on my system, either because the Drupal.behaviors.ucCountrySelect has not yet been called, or because the internal change does not generate a change event. When I trace the code, is looks like Drupal.behaviors.ucCountrySelect is executed first.

After returning, the ucAddressCountrySelect-processed class is added. I do not think this is necessary.

When the Drupal.behaviors.ucCountrySelect behavior gets called (which I actually think happens first), it adds a change() callback handler and then immediately adds the class ucCountrySelect-processed. This is the real reason both class names appear. However, it's inner function is not activated until a change event occurs.

It also seems to me that the added class is not needed. I am guessing the Drupal behaviors are executed only once after the page is loaded. In any case, I was able to comment out both class name additions without causing any obvious problems.

uc_addresses can disable filling in the delivery or billing addresses (or both). The code in your version of uc_addresses.js does not differentiate, as far as I can tell. uc_update_zone_select is called twice (once for each address), with the same default zone each time. This seems wrong, but the code worked correctly anyway and I'm not sure why. I even tried making sure that the default address was the same and different from the default country and it stilled worked. Anyway, it's getting late—I'll try to figure this out later.

The reason I want to understand this is to ensure that there isn't some subtle bug still left. Some of the things I was worrying about was that the order in which the behaviors are called may not be guaranteed and might cause a problem in some configurations or at some future time. I was also worried about any effects on Add/Edit, but they seem to be working great WITHOUT the need to set anything up for them using Javascript. It looks like I can junk the call to load uc_addresses.js for those forms. I am still a little dubious of calling uc_update_zone_select() for both billing and delivery when only one of them is filled in.

Thanks again for your help.