hook_block_configure not AHAH compatible

#3: 690828-block-ahah.patch queued for re-testing.

+++ modules/block/block.admin.inc	22 Jul 2010 18:59:22 -0000
@@ -583,3 +583,27 @@ function template_preprocess_block_admin
+ *   The block delta for this configuration.
+ * @return

Missing line between @param and @return

+++ modules/block/block.admin.inc	22 Jul 2010 18:59:22 -0000
@@ -583,3 +583,27 @@ function template_preprocess_block_admin
+ ¶

Trailing whitespace

+++ modules/block/block.api.php	22 Jul 2010 18:59:22 -0000
@@ -91,6 +91,9 @@ function hook_block_info_alter(&$blocks,
+ *   The current form state. Required for AHAH callbacks to function
+ *   properly.

I don't know if the second sentence is actually needed or if it would actually confuse people.

Also marked #1117590: Pass $form_state to block configure forms as duplicate.

Powered by Dreditor.

Note that $form_state must be the first parameter passed to the hook; this is because you cannot set a default value on a variable passed by reference.

As far as I know, that's not true.

Could we find a way to keep the signature of hook_block_configure() compatible? That's the only way to eventually get this into D7.

This issue is about AHAH, but there are non-AHAH use-cases for wanting $form_state in hook_block_configure() as well. See #430886-114: Make all blocks fieldable entities.

Here's a reroll with $form_state as last parameter.

I would like to challenge that. We would basically be testing module_invoke(), except that we can't use that because of the reference thing, so we roll our own.

webchick's or Dries' call, though, and if desired and I get to it, I'll roll some tests.

Thinking about this, the lack of a block_test_configure() shows that blocks are completely untested anyway. Let's not fix that here, please. I posted #1168644: Test hook_block_info() and friends for that. Removing tag.

I rolled some former version of this patch, but I'm going to mark it RTBC anyway. It's a super simple fix. As said above tests for this would be somewhat redundant, but more importantly complete scope-creep for this issue.

Adding tests for a bug report isn't scope creep, it's adding tests for a bug report.

This looks easy enough to test to me so we ought to be able to do it here.

As I pointed out above hook_block_configure() and friends are completely untested as it stands currently. So writing tests doesn't just mean testing this particular bug, but writing at least basic tests for hook_block_info(), hook_block_configure(), hook_block_save() and hook_block_view(). If someone wants to do that, all the better, but I don't think we should hold this up on that.
But that's just me. :)

This got fixed by #1535868: Convert all blocks into plugins. See http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/block/l...

Actually "closed (duplicate)" seems more fitting.

it would still be cool to fix this in d7

Let's see if #14 applies to D7.

Re-roll the patch.