Returning "An illegal choice has been detected. Please contact the site administrator" when changing country
inocram - February 19, 2009 - 09:27
| Project: | Ubercart Addresses |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | freixas |
| Status: | needs work |
Description
When adding or editing an address, if you change the country "An illegal choice has been detected. Please contact the site administrator" error is being returned.
Drupal generates this error because the form being submitted is different from the form on the cache. When you submit a form, drupal compares the submitted against the one on the cache but since the form has been altered (zones) when you select a different country the form doesn't match and thus generates the error.
The patch simply retrieves the cached form and then update the zones depending on the country selected then saves the cache back.

#1
I was able to attach the patch but for some reason it doesn't show up, heres the link: http://drupal.org/files/uc_addresses.patch
#2
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?
#3
Sorry for the late reply, perhaps the patch was just temporarily stored.
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.
#4
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().
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!
#5
Well, when I did this patch I wasn't using your latest update, I would also love to see what your going to come-up with. =)
#6
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.
#7
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.