"warning: array_keys" after EDIT and SAVE A BOOK

I got this Error Messages in version 6.2 while I deleted all pages of a book and then saving the book pages. I can report these errors:

* warning: array_keys() [function.array-keys]: The first argument should be an array in /mnt/workspace/ikd/www/modules/book/book.admin.inc on line 95.
* warning: array_flip() [function.array-flip]: The argument should be an array in /mnt/workspace/ikd/www/modules/book/book.admin.inc on line 95.
* warning: array_merge() [function.array-merge]: Argument #1 is not an array in /mnt/workspace/ikd/www/modules/book/book.admin.inc on line 96.

I assume this is the same type of error.

Changed to Version 6.2 because I got this with 6.2 and it is still not solved.

bugs against HEAD (7.x). Don't assign to yourself unless you are fixing it.

Created a patch against 6.4. The problem can be easily fixed, the error messages appear while the book is saved with no pages. A check for that was missing:

Index: drupal-root/modules/book/book.admin.inc
===================================================================
--- drupal-root/modules/book/book.admin.inc	(revision 38)
+++ drupal-root/modules/book/book.admin.inc	(working copy)
@@ -104,6 +104,12 @@
 function book_admin_edit_submit($form, &$form_state) {
   // Save elements in the same order as defined in post rather than the form.
   // This ensures parents are updated before their children, preventing orphans.
+  
+  if (false == array_key_exists('table', $form['#post'])) {
+  	drupal_set_message(t('Nothing to update in book %title.', array('%title' => $form['#node']->title)));
+  	return;
+  }
+	
   $order = array_flip(array_keys($form['#post']['table']));
   $form['table'] = array_merge($order, $form['table']);

Added patch as attachment, might make things easier.

Thanks for the insight into the bug. I'd probably write:

 if (false == array_key_exists('table', $form['#post'])) {

as

 if (empty($form['#post']['table'])) {

however, we might also be able to attack this by setting up better defaults in the form.

Empty checks for an empty string or a 0. I can not see why this should lead to something. I guess you mean Isset();.

empty also checks for !isset, empty array, etc. !isset() would be reasonable too, and would match exactly your logic but in a different form.

Thanks for the insight, I was not that aware about empty. Then I might give Isset() a go because it more clearly matches my intention in the fix and is faster then array_key_exists(). The patch needs a change then.

Just passing by ;-) but notice the following; The patch attached to this issue is currently for 6.4 while the issue is flagged as 7.x and hence show up in the "Patches to review (D7)" link on frontpage - witch is kinda misleading. So shouldn't this patch really be done for 7.x and then back ported to 6.x?

just chiming in with my eyes

ok no reason really I cannot provide the patch so here it is, built against latest HEAD (7.x).

Thanks for the patch, I ran out of time the last days. Please check the following:

$order = array_flip(array_keys($form['#post']['table']));
$form['table'] = array_merge($order, $form['table']);

this code is executed before you check that $form['#post']['table'] actually is not empty. but only afterwards the check this could be assumed. so therefore the added checks have to be lifted above these lines, shouldn't they?

I dunno wether or not these routines need to be very pedantic in their checks (i normally love that) so some infos would be nice for me to know. possible additionally checks could be:

* is $form an array?
* is $form['#post'] empty?

My bad, correct patch attached against latest HEAD.

Rerolled against latest 7.x. The function is exactly the same than in D6.

#13: book.admin_.inc_.2.patch queued for re-testing.

Attached updated patches for D6 and D7. This time they should complete the tests.

No longer applies.