Checkout form fails if button labels are altered [#1170364]

The text used to label the checkout buttons has been coded into the logic of the module, and so changing the button labels using hook_form_alter() result in a cart that will not complete the checkout process.

The following code in function uc_cart_view_form_submit

  // Specify the appropriate redirect based on the button used to submit.
  switch ($form_state['values']['op']) {
    // Continue shopping button.
    case $form_state['values']['continue_shopping_text']:
      $form_state['redirect'] = uc_cart_continue_shopping_url();
      break;

    // Update cart button.
    case t('Update cart'):
      // No redirect.  Just display a message and preserve the last URL.
      drupal_set_message(t('Your cart has been updated.'));
      break;

    // Checkout button.
    case t('Checkout'):
      $form_state['redirect'] = variable_get('uc_checkout_enabled', TRUE) ? 'cart/checkout' : 'cart';
      break;
  }

should be changed to use the values that each button label actually had, like so:

  // Specify the appropriate redirect based on the button used to submit.
  switch ($form_state['values']['op']) {
    // Continue shopping button.
    case $form_state['values']['continue_shopping_text']:
      $form_state['redirect'] = uc_cart_continue_shopping_url();
      break;

    // Update cart button.
    case $form_state['values']['update']:
      // No redirect.  Just display a message and preserve the last URL.
      drupal_set_message(t('Your cart has been updated.'));
      break;

    // Checkout button.
    case $form_state['values']['checkout']:
      $form_state['redirect'] = variable_get('uc_checkout_enabled', TRUE) ? 'cart/checkout' : 'cart';
      break;
  }

There may well be other places through the code where button labels have been fixed in the code and so present problems if hook_form_alter() is used to amend the form prior to display.

Cheers.

Comment	File	Size	Author
#13	1170364-uc_cart_view_form_submit.patch	2.78 KB	TR
#13
#8	1170364-uc_cart_view_form_submit_d6.patch	2.78 KB	hles
#8
#3	1170364-uc_cart_view_form_submit.patch	2.99 KB	longwave
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

longwave

he/him

English

CreditAttribution: longwave commented 27 May 2011 at 07:29

Title:

Cart Checkout Button Labels Fixed

» Checkout form fails if button labels are altered

Really, we should split that function into individual #submit handlers for each button, and do away with that switch() statement altogether.

Comment #2

TR CreditAttribution: TR commented 23 July 2011 at 15:59

Version:

6.x-2.4

» 7.x-3.x-dev

Lets fix this in 7 first.

Marked #1225406: Changing the #value of the checkout button the cart form means you can checkout as a duplicate.

Comment #3

longwave

he/him

English

CreditAttribution: longwave commented 23 July 2011 at 18:59

Status:

Active

» Needs review

File	Size
1170364-uc_cart_view_form_submit.patch	2.99 KB

Comment #4

nardberjean CreditAttribution: nardberjean commented 26 July 2011 at 14:46

The patch #3 #1170364: Checkout form fails if button labels are altered has been applyied mostly successfully to issue #1228418-3: 'Continue Shopping' button stays on cart page.

Comment #5

longwave

he/him

English

CreditAttribution: longwave commented 26 July 2011 at 19:09

Status:

Needs review

» Fixed

Committed #3.

Comment #6

longwave

he/him

English

CreditAttribution: longwave commented 26 July 2011 at 19:09

Version:	7.x-3.x-dev	» 6.x-2.x-dev
Status:	Fixed	» Patch (to be ported)

Comment #7

TR CreditAttribution: TR commented 24 August 2011 at 01:26

Comment #8

hles CreditAttribution: hles commented 25 August 2011 at 13:45

File	Size
1170364-uc_cart_view_form_submit_d6.patch	2.78 KB

Backport against ubercart 6.x latest dev.

Comment #9

hles CreditAttribution: hles commented 26 August 2011 at 08:32

We also have the same problem with the cancel button of the checkout page in uc_cart_checkout_form:

  // Cancel an order when a customer clicks the 'Cancel' button.
  if (isset($_POST['op']) && $_POST['op'] == t('Cancel')) {
    if (isset($_SESSION['cart_order']) && intval($_SESSION['cart_order']) > 0) {
      uc_order_comment_save($_SESSION['cart_order'], 0, t('Customer cancelled this order from the checkout form.'));
      unset($_SESSION['cart_order']);
    }
    drupal_goto('cart');
  }

Comment #10

hles CreditAttribution: hles commented 26 August 2011 at 08:32

Status:

Patch (to be ported)

» Needs work

Comment #11

hles CreditAttribution: hles commented 26 August 2011 at 09:14

I am not sure about the purpose of the cancel button. Someone may want to come back to the cart page in order to change a quantity or remove a product, in that case the order is actually not cancelled, just "delayed" in some way. And the Cancel button would then become a link instead of a "false" submit button.
Else the cancel button could be part of a confirm_form() function instead.
In both cases, it would solve the problem of this thread for this button.

Comment #12

longwave

he/him

English

CreditAttribution: longwave commented 5 October 2011 at 16:24

Status:

Needs work

» Needs review

Needs review in 6.x based on #8.

Comment #13

TR CreditAttribution: TR commented 6 October 2011 at 07:14

File	Size
1170364-uc_cart_view_form_submit.patch	2.78 KB

Re-uploading patch from #8 and removing the _d6 from the name so the testbot won't ignore it. I didn't change the patch at all, just the name.

Comment #14

longwave

he/him

English

CreditAttribution: longwave commented 6 October 2011 at 15:49

@TR: testbot is choking on the 6.x-2.x branch for some reason I don't quite understand: http://qa.drupal.org/pifr/test/132139

Comment #15

TR CreditAttribution: TR commented 6 October 2011 at 16:44

Yup, I saw. I don't know either, since the branch runs clean locally. I'll work with rfay to figure it out. In the meantime patches like the above will remain stalled until the branch is restarted.

I've applied and tested the patch locally and it runs green. But I haven't looked at it or tried it out to see that it does the same thing as the 7.x patch that was committed.

Comment #16

TR CreditAttribution: TR commented 8 October 2011 at 05:06

The 5 fails on the Order tests were due to the testbot running a different version of simpletest - testbot uses 6.x-2.x-dev, which has changes in the way #disabled form elements are handled. That's fixed now, and I also fixed the 9 exceptions, but I can't reproduce the remaining 2 fails in the Cart and checkout tests. My only remaining theory is that they show up only in PHP 5.3 (used by the testbot). My machines are currently running 5.2, so I haven't been able to check this yet. The 6.x-2.x branch tests will remain stalled until this is corrected.