17 replies [Last post]
adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Was this information Helpful?

Normally when a form is submitted with missing or incorrect information, an error is displayed at the top of the page, and the form is displayed again with the problem fields highlighted.

This doesn't happen for forms loaded dynamically on the checkout form, such as the credit card form. Is there any way I can make this happen? I don't care if it's black magic, as long as it doesn't include hacking core.

Thanks.

CKoch's picture
Offline
Joined: 05/29/2009
Juice: 10
Re: Checkout form: Highlight fields with errors in dynamically

Did you find a means of doing this?

pfrilling's picture
Offline
Joined: 01/06/2009
Juice: 36
Re: Re: Checkout form: Highlight fields with errors in dynamica

I'm looking for this functionality currenty. Does anyone know how to do this?

TR
TR's picture
Online
Bug FinderFAQ ModeratorGetting busy with the Ubercode.
Joined: 11/05/2007
Juice: 3424
Re: Re: Re: Checkout form: Highlight fields with errors in dyna

Validating a form is handled at several levels. First is the default validations handled by Drupal, such as checking that fields marked "required" have had values entered. For this type of validation, fields with errors ARE highlighted, even if they are in dynamically loaded checkout panes.

The problem seems to be with just certain panes, where fields which are NOT marked as required are subjected to pane-specific validation functions which don't call form_set_error() when the validation fails. An example is the credit card payment method, where the card number and CVV fields are not marked as required, but should be IMO. Or if for some reason they were intentionally left as not-required (and I can't think of a good one ...) then the pane validation function should be calling form_set_error().

As a work-around, you can do a hook_form_alter() of the checkout form to set those fields as required, or you can write your own validation function and use hook_form_alter() to add it to the array of validation functions. Or you could write a patch to add calls to form_set_error() to all the checkout panes where you see this problem and submit it to the issue queue at drupal.org/project/ubercart.

<tr>.
adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Re: Re: Re: Re: Checkout form: Highlight fields with errors in

You are correct that the uc_credit module doesn't use form_set_error(). It uses drupal_set_message().

However, even if I use form_set_error() to set an error on the cc_number field, the field does not get highlighted. Even if I set the cc_number field to required and submit the form with cc_number blank, the field does not get highlighted.

I've never seen anything get highlighted in any of the dynamically loaded panes. As I understand it, form_set_error just adds the message to a static array and calls drupal_set_message. Then when forms are built the names of the form elements are checked against the static array and the fields are highlighted. By the dynamically loaded form panes are loaded via a separate HTTP request, and so the static variable won't be populated with the messages that were set during the last HTTP request. I'd love to be proven wrong.

adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Re: Re: Re: Re: Re: Checkout form: Highlight fields with errors

I think I have an idea how this can work, maybe...

1) Use hook_form_alter() on the checkout form to get the output of form_set_error() when the checkout form is built.
2) Grab errors for any dynamically loaded panes out of that and store the data in the $_SESSION variable.
3) Use hook_form_alter() on any dynamically loaded pane forms to check the $_SESSION variable for errors in that form.
4) Call form_set_error() for each and then unset that slice of the $_SESSION variable.

I haven't tried it yet but something like that should work. I'll post back when I get a chance to test it out.

TR
TR's picture
Online
Bug FinderFAQ ModeratorGetting busy with the Ubercode.
Joined: 11/05/2007
Juice: 3424
Re: Re: Re: Re: Re: Checkout form: Highlight fields with errors

I don't know about the credit module specifically, but I've written my own validation function as I described in #3 several times, with success, on the address panes and on a custom payment method pane. In principle, all the panes should be similar.

I just looked at a print_r($form) on the checkout page, and boy is it ... umm, let's call it "suboptimal". For example, the field "delivery_street1" appears no less than 57 times (!!!) in that array. I remember found it hard to choose the right element to form_set_error() on when wrote my own validation function.

adamo wrote:

I've never seen anything get highlighted in any of the dynamically loaded panes.

You've never seen one of the required fields in the address highlighted when it was left empty? Then you may have a problem with your theme's CSS.

<tr>.
TR
TR's picture
Online
Bug FinderFAQ ModeratorGetting busy with the Ubercode.
Joined: 11/05/2007
Juice: 3424
adamo wrote: However, even
adamo wrote:

However, even if I use form_set_error() to set an error on the cc_number field, the field does not get highlighted.

I just did a quick test where I added
form_set_error('cc_number', t("Card Number is bogus!"));
as the last line in the function uc_payment_method_credit_form(), just before the "return $form;".

On form submission, I was returned to cart/checkout with the cc_number filed highlighted as expected and with the error message at the top of the page.

<tr>.
adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Re: adamo wrote: However, even

It works fine for any pane that isn't dynamically loaded. Address panes and anything else loaded in the initial checkout form have their fields highlighted as expected. It's only a problem with the ones that get loaded via AHAH or AJAX or whatever it is after the initial checkout form is loaded.

My code is definitely executing form_set_error('cc_number', $message). I stepped through it in a debugger. Are you using D6/UC2? I'm on D6/UC2-RC6.

TR
TR's picture
Online
Bug FinderFAQ ModeratorGetting busy with the Ubercode.
Joined: 11/05/2007
Juice: 3424
Re: Re: adamo wrote: However, even

I tested on RC3.

<tr>.
adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Re: Re: Re: adamo wrote: However, even

Very strange. I've had this problem since well before RC3. Is your payment pane loaded with the initial checkout form or does it get loaded after via AHAH/AJAX/whatever?

TR
TR's picture
Online
Bug FinderFAQ ModeratorGetting busy with the Ubercode.
Joined: 11/05/2007
Juice: 3424
Re: Re: Re: Re: adamo wrote: However, even

My test in #7 was with just the Credit module enabled, so the payment pane shows up immediately on page load.

However, I also tested it with two payment methods enabled: Credit and my custom eCheck module. They both accept different user information: eCheck needs bank account number etc, Credit needs card number etc. Because the eCheck module has the lower weight, its input fields are displayed by default in the payment pane. But then when the Credit radio button is selected the uc_payment module uses AHAH to load and display the fields for the Credit module and to remove the fields for the eCheck module.

With my mod in #7, submitting the cart/checkout form gives me an error and highlighting in both of these tests.

<tr>.
adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
OK this is very helpful.

OK this is very helpful. Smiling If I'm understanding you correctly, the default payment pane should be loaded along with the main checkout form. It shouldn't use AHAH to load the payment pane until the user changes their payment selection. Correct?

I think my problem is that it is always using AHAH to load the payment pane, so it is effectively loading it twice. Now if I can just figure out why the heck that is happening.

Thanks a ton!

edit: Hmmm... maybe I'm not understanding you correctly... UC seems to ALWAYS load the payment details via AHAH, and NEVER load them along with the main checkout form. If I set a breakpoint in the uc_payment_method_credit_form function, it doesn't get hit when the initial checkout form is loaded. It only gets hit when the payment details are loaded via AHAH after the main checkout form has been loaded. Have you done something special to get the payment details included in the main checkout form?

adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Are you messing with me?

Are you messing with me? Eye-wink

The default payment method definitely does not get loaded with the initial checkout form. It's always dynamically loaded. Even if I manually apply this patch: http://drupal.org/node/439322#comment-2006124, to include the default payment method in the main checkout form the fields do not get highlighted, because the validation code is not called until after the form has already been rendered (wtf?). What a nightmare.

TR
TR's picture
Online
Bug FinderFAQ ModeratorGetting busy with the Ubercode.
Joined: 11/05/2007
Juice: 3424
Re: OK this is very helpful.

I think you're right that the payments pane is loaded dynamically - at least I can see the AJAX (AHAH?) call in Firebug when the cart/checkout page is loaded. But it gets loaded immediately - it doesn't need to wait for any address information or anything like that. Only when you have multiple payment methods will the content of the payment pane change in response to user input. That is, selecting a payment method from a list of radio buttons on cart/checkout will change the pane content to reflect the information needed from the customer for that method.

Nevertheless, I tested both a single payment method (Credit card) as well as multiple methods (Credit card + custom), and the highlighting worked in both cases.

<tr>.
adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
Re: Re: OK this is very helpful.

Gotcha. That's exactly the behavior I'm seeing (after reverting back to the lastest dev snapshot). Except, none of the payment method fields ever get highlighted for me.

OK I just noticed you said you put form_set_error in uc_payment_method_credit_form. That DOES work. But you are setting the error right after the form is built, every time the form is built. That's not where submitted form data gets validated. It gets validated in the cart-process op of uc_payment_method_credit(). Try putting form_set_error in there and see if the form gets highlighted.

So we've come full circle back to what I said in #4:

Quote:

As I understand it, form_set_error just adds the message to a static array and calls drupal_set_message. Then when forms are built the names of the form elements are checked against the static array and the fields are highlighted. By the dynamically loaded form panes are loaded via a separate HTTP request, and so the static variable won't be populated with the messages that were set during the last HTTP request.

It looks like storing the errors in the $_SESSION variable is the only viable way around this right now. I wish I'd read your post more carefully the first time. *slaps forehead*

adamo's picture
Offline
Getting busy with the Ubercode.
Joined: 02/17/2009
Juice: 229
I found a way. In your

I found a way.

In your validation code, use form_set_error like normal. Then at the very end of your validation function just before the return, put something like this:

  // Store form errors in $_SESSION variable so when can set error class on appropriate fields
  //when form is loaded dynamically.
  $_SESSION['modulename']['form_errors'] = form_get_errors();

Then, at the very end of your form generation code, just before the return, put something like this:

  // This is necessary so that fields with errors will be properly highlighted.
  $form_errors = $_SESSION['modulename']['form_errors'];
  if (is_array($form_errors)) {
    foreach ($form_errors as $k => $v) {
      if (array_key_exists($k, $form)) {
        $form[$k]['#attributes'] = array('class' => 'error');
      }
    }
  }
  unset($_SESSION['modulename']['form_errors']);

This hasn't been thoroughly tested, but seems to be working fine for me. Yay!

KingAndy's picture
Offline
Joined: 04/01/2009
Juice: 95
Re: I found a way. In your

Has this, or anything like it, ever been folded into Ubercart core (or at least the relevent payment modules)?

Could it?

For the sake of argument: Should it? (I think so, it's a fairly big flaw in the checkout process if validation isn't highlighting in line with Drupal standards...)