undefined indices..

Good catches, thanks.

1. +  if (!isset($_SESSION['captcha_success_form_ids'])) {
   +    $_SESSION['captcha_success_form_ids'] = array();
   +  }
      $captcha_success_form_ids = (array)($_SESSION['captcha_success_form_ids']);
   

   Good point, but I would do this like

      $captcha_success_form_ids = isset($_SESSION['captcha_success_form_ids']) ? (array)($_SESSION['captcha_success_form_ids']) : array();
   

   to avoid writing to $_SESSION when not needed.

2. -    'solution' => $captcha['solution'],
   +    'solution' => isset($captcha['solution']) ? $captcha['solution'] : '',
   

   this looks strange. with what challenge module did you get an undefined index warning here?
   Anyway, if the solution is not set, I, think its a bit dangerous to leave it empty, as if the empty answer is the right one. I'll have to look into this.

3. -  $csid = $form_state['clicked_button']['#post']['captcha_sid'];
   +  $csid = isset($form_state['clicked_button']['#post']['captcha_sid']) ? $form_state['clicked_button']['#post']['captcha_sid'] : 0;
    

   On which form did you get an undefined index warning here?

reproduced problem 1) and committed fix in http://drupal.org/cvs?commit=256840, thanks

concerning 2) and 3) I really would like more info on how to reproduce. Your explanation in #2 is a bit fuzzy :) I immediately don't see how "hickups" and aborted page calls could cause "undefined indices" messages that would not show up on normal page calls in those cases.

For example:
for 2) which CAPTCHA type are you using?
for 3) on which form_ids do you have a CAPTCHA?

3) is related to #534168: Submit with "return"/"enter" key in IE7 leads to failing CAPTCHA and it will be solved over there

The remaining thing is 2):

-    'solution' => $captcha['solution'],
+    'solution' => isset($captcha['solution']) ? $captcha['solution'] : '',

I'd like instruction on how to reproduce this because if $captcha['solution'] is not set, something is seriously wrong. Ignoring this by just blindly setting the solution to an empty string is a bad idea IMHO.

Can you describe what bug the patch is trying to fix?

-    if (!$captcha) {
+    if (empty($captcha)) {

Why this change? According to the PHP documentation (http://www.php.net/manual/en/function.empty.php, http://php.net/manual/en/language.types.boolean.php), there is not much difference in this context ($captcha should be an array).

 
+    // Store information for usage in the validation and pre_render phase.
+    $element['#captcha_info'] = array(
+      'form_id' => $this_form_id,
+      'module' => $captcha_type_module,
+      'type' => $captcha_type_challenge,
+      'captcha_sid' => $captcha_sid,
+      'solution' => $captcha['solution'],
+    );
   }
 
-  // Store information for usage in the validation and pre_render phase.
-  $element['#captcha_info'] = array(
-    'form_id' => $this_form_id,
-    'module' => $captcha_type_module,
-    'type' => $captcha_type_challenge,
-    'captcha_sid' => $captcha_sid,
-    'solution' => $captcha['solution'],
-  );
-

Why this move? What is the bug here?

the $element['#captcha_info'] stuff is outside the if body, because that information could also be used by other "consumers" than the validate and prerender handlers. The documentation does not mention that, so I understand the misunderstanding :). Apart from that, 'solution' => $captcha['solution'] is indeed undefined when the if condition evaluates to false, so can cause "undefined indices" problems.

Anyway, I worked a bit on your patch

reroll
(and hopefully the testbot will pick this up)

http://drupal.org/cvs?commit=342818
http://drupal.org/cvs?commit=342832