- Patch #345866 by alexanderpas, justinrandell, Dave Reid: remove from hook_block().

5 files changed, 233 insertions, 148 deletions

diff --git a/modules/block/block.admin.inc b/modules/block/block.admin.inc
index 2b7dbda28..ead47fba0 100644
--- a/modules/block/block.admin.inc
+++ b/modules/block/block.admin.inc
@@ -171,14 +171,14 @@ function block_admin_configure(&$form_state, $module = NULL, $delta = 0) {
   );
 
   // Module-specific block configurations.
-  if ($settings = module_invoke($module, 'block', 'configure', $delta)) {
+  if ($settings = module_invoke($module, 'block_configure', $delta)) {
     foreach ($settings as $k => $v) {
       $form['block_settings'][$k] = $v;
     }
   }
 
   // Get the block subject for the page title.
-  $info = module_invoke($module, 'block', 'list');
+  $info = module_invoke($module, 'block_list');
   if (isset($info[$delta])) {
     drupal_set_title(t("'%name' block", array('%name' => $info[$delta]['info'])), PASS_THROUGH);
   }
@@ -285,7 +285,7 @@ function block_admin_configure_submit($form, &$form_state) {
     foreach (array_filter($form_state['values']['roles']) as $rid) {
       db_query("INSERT INTO {block_role} (rid, module, delta) VALUES (%d, '%s', '%s')", $rid, $form_state['values']['module'], $form_state['values']['delta']);
     }
-    module_invoke($form_state['values']['module'], 'block', 'save', $form_state['values']['delta'], $form_state['values']);
+    module_invoke($form_state['values']['module'], 'block_save', $form_state['values']['delta'], $form_state['values']);
     drupal_set_message(t('The block configuration has been saved.'));
     cache_clear_all();
     $form_state['redirect'] = 'admin/build/block';
diff --git a/modules/block/block.api.php b/modules/block/block.api.php
index 57517dbc2..72a2c6657 100644
--- a/modules/block/block.api.php
+++ b/modules/block/block.api.php
@@ -12,87 +12,85 @@
  */
 
 /**
- * Declare a block or set of blocks.
+ * List of all blocks defined by the module.
  *
  * Any module can export a block (or blocks) to be displayed by defining
  * the _block hook. This hook is called by theme.inc to display a block,
  * and also by block.module to procure the list of available blocks.
  *
- * @param $op
- *   What kind of information to retrieve about the block or blocks.
- *   Possible values:
- *   - 'list': A list of all blocks defined by the module.
- *   - 'configure': Configuration form for the block.
- *   - 'save': Save the configuration options.
- *   - 'view': Process the block when enabled in a region in order to view its contents.
- * @param $delta
- *   Which block to return (not applicable if $op is 'list'). This is a
- *   descriptive string used to identify blocks within each module and also
- *   within the theme system. The $delta for each block is defined within
- *   the array that your module returns when $op is 'list' (see below).
- * @param $edit
- *   If $op is 'save', the submitted form data from the configuration form.
  * @return
- *   - If $op is 'list': An associative array whose keys define the $delta
- *     for each block and whose values contain the block descriptions. Each
- *     block description is itself an associative array, with the following
- *     key-value pairs:
- *     - 'info': (required) The human-readable name of the block.
- *     - 'cache': A bitmask of flags describing how the block should behave with
- *       respect to block caching. The following shortcut bitmasks are provided
- *       as constants in block.module:
- *       - BLOCK_CACHE_PER_ROLE (default): The block can change depending on the
- *         roles the user viewing the page belongs to.
- *       - BLOCK_CACHE_PER_USER: The block can change depending on the user
- *         viewing the page. This setting can be resource-consuming for sites
- *         with large number of users, and should only be used when
- *         BLOCK_CACHE_PER_ROLE is not sufficient.
- *       - BLOCK_CACHE_PER_PAGE: The block can change depending on the page
- *         being viewed.
- *       - BLOCK_CACHE_GLOBAL: The block is the same for every user on every
- *         page where it is visible.
- *       - BLOCK_NO_CACHE: The block should not get cached.
- *     - 'weight', 'status', 'region', 'visibility', 'pages':
- *       You can give your blocks an explicit weight, enable them, limit them to
- *       given pages, etc. These settings will be registered when the block is first
- *       loaded at admin/block, and from there can be changed manually via block
- *       administration.
- *       Note that if you set a region that isn't available in a given theme, the
- *       block will be registered instead to that theme's default region (the first
- *       item in the _regions array).
- *   - If $op is 'configure': optionally return the configuration form.
- *   - If $op is 'save': return nothing.
- *   - If $op is 'view': return an array which must define a 'subject' element
- *     and a 'content' element defining the block indexed by $delta.
- *
- * The functions mymodule_display_block_exciting and _amazing, as used in the
- * example, should of course be defined somewhere in your module and return the
- * content you want to display to your users. If the "content" element is empty,
- * no block will be displayed even if "subject" is present.
+ *   An associative array whose keys define the $delta
+ *   for each block and whose values contain the block descriptions. Each
+ *   block description is itself an associative array, with the following
+ *   key-value pairs:
+ *   - 'info': (required) The human-readable name of the block.
+ *   - 'cache': A bitmask of flags describing how the block should behave with
+ *     respect to block caching. The following shortcut bitmasks are provided
+ *     as constants in block.module:
+ *     - BLOCK_CACHE_PER_ROLE (default): The block can change depending on the
+ *       roles the user viewing the page belongs to.
+ *     - BLOCK_CACHE_PER_USER: The block can change depending on the user
+ *       viewing the page. This setting can be resource-consuming for sites
+ *       with large number of users, and should only be used when
+ *       BLOCK_CACHE_PER_ROLE is not sufficient.
+ *     - BLOCK_CACHE_PER_PAGE: The block can change depending on the page
+ *       being viewed.
+ *     - BLOCK_CACHE_GLOBAL: The block is the same for every user on every
+ *       page where it is visible.
+ *     - BLOCK_NO_CACHE: The block should not get cached.
+ *   - 'weight', 'status', 'region', 'visibility', 'pages':
+ *     You can give your blocks an explicit weight, enable them, limit them to
+ *     given pages, etc. These settings will be registered when the block is first
+ *     loaded at admin/block, and from there can be changed manually via block
+ *     administration.
+ *     Note that if you set a region that isn't available in a given theme, the
+ *     block will be registered instead to that theme's default region (the first
+ *     item in the _regions array).
  *
  * After completing your blocks, do not forget to enable them in the
  * block admin menu.
  *
  * For a detailed usage example, see block_example.module.
  */
-function hook_block($op = 'list', $delta = '', $edit = array()) {
-  if ($op == 'list') {
-    $blocks['exciting'] = array(
-      'info' => t('An exciting block provided by Mymodule.'),
-      'weight' => 0,
-      'status' => 1,
-      'region' => 'left',
-      // BLOCK_CACHE_PER_ROLE will be assumed for block 0.
-    );
+function hook_block_list() {
+  $blocks['exciting'] = array(
+    'info' => t('An exciting block provided by Mymodule.'),
+    'weight' => 0,
+    'status' => 1,
+    'region' => 'left',
+    // BLOCK_CACHE_PER_ROLE will be assumed for block 0.
+  );
 
-    $blocks['amazing'] = array(
-      'info' => t('An amazing block provided by Mymodule.'),
-      'cache' => BLOCK_CACHE_PER_ROLE | BLOCK_CACHE_PER_PAGE,
-    );
+  $blocks['amazing'] = array(
+    'info' => t('An amazing block provided by Mymodule.'),
+    'cache' => BLOCK_CACHE_PER_ROLE | BLOCK_CACHE_PER_PAGE,
+  );
 
-    return $blocks;
-  }
-  elseif ($op == 'configure' && $delta == 'exciting') {
+  return $blocks;
+}
+
+/**
+ * Configuration form for the block.
+ *
+ * Any module can export a block (or blocks) to be displayed by defining
+ * the _block hook. This hook is called by theme.inc to display a block,
+ * and also by block.module to procure the list of available blocks.
+ *
+ * @param $delta
+ *   Which block to return. This is a descriptive string used to identify 
+ *   blocks within each module and also within the theme system.
+ *   The $delta for each block is defined within the array that your module
+ *   returns when the hook_block_list() implementation is called.
+ * @return
+ *   Optionally return the configuration form.
+ *
+ * After completing your blocks, do not forget to enable them in the
+ * block admin menu.
+ *
+ * For a detailed usage example, see block_example.module.
+ */
+function hook_block_configure($delta = '') {
+  if ($delta == 'exciting') {
     $form['items'] = array(
       '#type' => 'select',
       '#title' => t('Number of items'),
@@ -101,26 +99,76 @@ function hook_block($op = 'list', $delta = '', $edit = array()) {
     );
     return $form;
   }
-  elseif ($op == 'save' && $delta == 'exciting') {
+}
+
+/**
+ * Save the configuration options.
+ *
+ * Any module can export a block (or blocks) to be displayed by defining
+ * the _block hook. This hook is called by theme.inc to display a block,
+ * and also by block.module to procure the list of available blocks.
+ *
+ * @param $delta
+ *   Which block to save the settings for. This is a descriptive string used
+ *   to identify blocks within each module and also within the theme system.
+ *   The $delta for each block is defined within the array that your module
+ *   returns when the hook_block_list() implementation is called.
+ * @param $edit
+ *   The submitted form data from the configuration form.
+ *
+ * After completing your blocks, do not forget to enable them in the
+ * block admin menu.
+ *
+ * For a detailed usage example, see block_example.module.
+ */
+function hook_block_save($delta = '', $edit = array()) {
+  if ($delta == 'exciting') {
     variable_set('mymodule_block_items', $edit['items']);
   }
-  elseif ($op == 'view') {
-    switch ($delta) {
-      case 'exciting':
-        $block = array(
-          'subject' => t('Default title of the exciting block'),
-          'content' => mymodule_display_block_exciting(),
-        );
-        break;
-      case 'amazing':
-        $block = array(
-          'subject' => t('Default title of the amazing block'),
-          'content' => mymodule_display_block_amazing(),
-        );
-        break;
-    }
-    return $block;
+}
+
+/**
+ * Process the block when enabled in a region in order to view its contents.
+ *
+ * Any module can export a block (or blocks) to be displayed by defining
+ * the _block hook. This hook is called by theme.inc to display a block,
+ * and also by block.module to procure the list of available blocks.
+ *
+ * @param $delta
+ *   Which block to return. This is a descriptive string used to identify 
+ *   blocks within each module and also within the theme system.
+ *   The $delta for each block is defined within the array that your module
+ *   returns when the hook_block_list() implementation is called.
+ * @return
+ *   An array which must define a 'subject' element and a 'content' element
+ *   defining the block indexed by $delta.
+ *
+ * The functions mymodule_display_block_exciting and _amazing, as used in the
+ * example, should of course be defined somewhere in your module and return the
+ * content you want to display to your users. If the "content" element is empty,
+ * no block will be displayed even if "subject" is present.
+ *
+ * After completing your blocks, do not forget to enable them in the
+ * block admin menu.
+ *
+ * For a detailed usage example, see block_example.module.
+ */
+function hook_block_view($delta = '') {
+  switch ($delta) {
+    case 'exciting':
+      $block = array(
+        'subject' => t('Default title of the exciting block'),
+        'content' => mymodule_display_block_exciting(),
+      );
+      break;
+    case 'amazing':
+      $block = array(
+        'subject' => t('Default title of the amazing block'),
+        'content' => mymodule_display_block_amazing(),
+      );
+      break;
   }
+  return $block;
 }
 
 /**
diff --git a/modules/block/block.install b/modules/block/block.install
index 585344c4e..a35170425 100644
--- a/modules/block/block.install
+++ b/modules/block/block.install
@@ -166,4 +166,11 @@ function block_schema() {
   $schema['cache_block']['description'] = 'Cache table for the Block module to store already built blocks, identified by module, delta, and various contexts which may change the block, such as theme, locale, and caching mode defined for the block.';
 
   return $schema;
-}
\ No newline at end of file
+}
+
+/**
+ * Refresh the block cache.
+ */
+function block_update_7000() {
+  return array();
+}
diff --git a/modules/block/block.module b/modules/block/block.module
index 641f1cdea..ed9c8c089 100644
--- a/modules/block/block.module
+++ b/modules/block/block.module
@@ -15,7 +15,7 @@ define('BLOCK_REGION_NONE', -1);
  * Constants defining cache granularity for blocks.
  *
  * Modules specify the caching patterns for their blocks using binary
- * combinations of these constants in their hook_block(op 'list'):
+ * combinations of these constants in their hook_block_list():
  *   $block[delta]['cache'] = BLOCK_CACHE_PER_ROLE | BLOCK_CACHE_PER_PAGE;
  * BLOCK_CACHE_PER_ROLE is used as a default when no caching pattern is
  * specified.
@@ -180,45 +180,52 @@ function _block_themes_access($theme) {
 }
 
 /**
- * Implementation of hook_block().
- *
- * Generates the administrator-defined blocks for display.
+ * Implementation of hook_block_list().
  */
-function block_block($op = 'list', $delta = 0, $edit = array()) {
-  switch ($op) {
-    case 'list':
-      $blocks = array();
-
-      $result = db_query('SELECT bid, info FROM {box} ORDER BY info');
-      while ($block = db_fetch_object($result)) {
-        $blocks[$block->bid]['info'] = $block->info;
-        // Not worth caching.
-        $blocks[$block->bid]['cache'] = BLOCK_NO_CACHE;
-      }
-      return $blocks;
-
-    case 'configure':
-      $box = array('format' => FILTER_FORMAT_DEFAULT);
-      if ($delta) {
-        $box = block_box_get($delta);
-      }
-      if (filter_access($box['format'])) {
-        return block_box_form($box);
-      }
-      break;
+function block_block_list() {
+  $blocks = array();
 
-    case 'save':
-      block_box_save($edit, $delta);
-      break;
+  $result = db_query('SELECT bid, info FROM {box} ORDER BY info');
+  while ($block = db_fetch_object($result)) {
+    $blocks[$block->bid]['info'] = $block->info;
+    // Not worth caching.
+    $blocks[$block->bid]['cache'] = BLOCK_NO_CACHE;
+  }
+  return $blocks;
+}
 
-    case 'view':
-      $block = db_fetch_object(db_query('SELECT body, format FROM {box} WHERE bid = %d', $delta));
-      $data['content'] = check_markup($block->body, $block->format, '', FALSE);
-      return $data;
+/**
+ * Implementation of hook_block_configure().
+ */
+function block_block_configure($delta = 0, $edit = array()) {
+  $box = array('format' => FILTER_FORMAT_DEFAULT);
+  if ($delta) {
+    $box = block_box_get($delta);
+  }
+  if (filter_access($box['format'])) {
+    return block_box_form($box);
   }
 }
 
 /**
+ * Implementation of hook_block_save().
+ */
+function block_block_save($delta = 0, $edit = array()) {
+  block_box_save($edit, $delta);
+}
+
+/**
+ * Implementation of hook_block_view().
+ *
+ * Generates the administrator-defined blocks for display.
+ */
+function block_block_view($delta = 0, $edit = array()) {
+  $block = db_fetch_object(db_query('SELECT body, format FROM {box} WHERE bid = %d', $delta));
+  $data['content'] = check_markup($block->body, $block->format, '', FALSE);
+  return $data;
+}
+
+/**
  * Update the 'block' DB table with the blocks currently exported by modules.
  *
  * @return
@@ -239,8 +246,8 @@ function _block_rehash() {
   // Valid region names for the theme.
   $regions = system_region_list($theme_key);
 
-  foreach (module_implements('block') as $module) {
-    $module_blocks = module_invoke($module, 'block', 'list');
+  foreach (module_implements('block_list') as $module) {
+    $module_blocks = module_invoke($module, 'block_list');
     if ($module_blocks) {
       foreach ($module_blocks as $delta => $block) {
         if (empty($old_blocks[$module][$delta])) {
@@ -349,7 +356,7 @@ function block_user_form(&$edit, &$account, $category = NULL) {
     $result = db_query("SELECT DISTINCT b.* FROM {block} b LEFT JOIN {block_role} r ON b.module = r.module AND b.delta = r.delta WHERE b.status = 1 AND b.custom != 0 AND (r.rid IN (" . db_placeholders($rids) . ") OR r.rid IS NULL) ORDER BY b.weight, b.module", $rids);
     $form['block'] = array('#type' => 'fieldset', '#title' => t('Block configuration'), '#weight' => 3, '#collapsible' => TRUE, '#tree' => TRUE);
     while ($block = db_fetch_object($result)) {
-      $data = module_invoke($block->module, 'block', 'list');
+      $data = module_invoke($block->module, 'block_list');
       if ($data[$block->delta]['info']) {
         $return = TRUE;
         $form['block'][$block->module][$block->delta] = array('#type' => 'checkbox', '#title' => check_plain($data[$block->delta]['info']), '#default_value' => isset($account->block[$block->module][$block->delta]) ? $account->block[$block->module][$block->delta] : ($block->custom == 1));
@@ -483,7 +490,7 @@ function _block_render_blocks($region_blocks) {
           $array = $cache->data;
         }
         else {
-          $array = module_invoke($block->module, 'block', 'view', $block->delta);
+          $array = module_invoke($block->module, 'block_view', $block->delta);
           if (isset($cid)) {
             cache_set($cid, $array, 'cache_block', CACHE_TEMPORARY);
           }
diff --git a/modules/block/block.test b/modules/block/block.test
index 0a203338c..f4a519d4e 100644
--- a/modules/block/block.test
+++ b/modules/block/block.test
@@ -2,6 +2,8 @@
 // $Id$
 
 class BlockTestCase extends DrupalWebTestCase {
+  protected $regions;
+
   function getInfo() {
     return array(
       'name' => t('Block functionality'),
@@ -16,6 +18,14 @@ class BlockTestCase extends DrupalWebTestCase {
     // Create and login user
     $admin_user = $this->drupalCreateUser(array('administer blocks', 'administer filters'));
     $this->drupalLogin($admin_user);
+
+    // Define the exising regions
+    $this->regions = array();
+    $this->regions[] = array('name' => 'header', 'id' => 'header-region');
+    $this->regions[] = array('name' => 'left', 'id' => 'sidebar-left');
+    $this->regions[] = array('name' => 'content', 'id' => 'center');
+    $this->regions[] = array('name' => 'right', 'id' => 'sidebar-right');
+    $this->regions[] = array('name' => 'footer');
   }
 
   /**
@@ -36,17 +46,12 @@ class BlockTestCase extends DrupalWebTestCase {
     // Check to see if the box was created by checking that it's in the database..
     $this->assertNotNull($bid, t('Box found in database'));
 
-    // Set the created box to a specific region.
-    // TODO: Implement full region checking.
-    $edit = array();
-    $edit['block_' . $bid . '[region]'] = 'left';
-    $this->drupalPost('admin/build/block', $edit, t('Save blocks'));
-
-    // Confirm that the box was moved to the proper region.
-    $this->assertText(t('The block settings have been updated.'), t('Box successfully moved to left region.'));
-
-    // Confirm that the box is being displayed.
-    $this->assertText(t($box['title']), t('Box successfully being displayed on the page.'));
+    // Check if the block can be moved to all availble regions.
+    $box['module'] = 'block';
+    $box['delta'] = $bid;
+    foreach ($this->regions as $region) {
+      $this->moveBlockToRegion($box, $region);
+    }
 
     // Delete the created box & verify that it's been deleted and no longer appearing on the page.
     $this->drupalPost('admin/build/block/delete/' . $bid, array(), t('Delete'));
@@ -94,17 +99,10 @@ class BlockTestCase extends DrupalWebTestCase {
     // Check to see if the block was created by checking that it's in the database.
     $this->assertNotNull($bid, t('Block found in database'));
 
-    // Set the created block to a specific region.
-    $edit = array();
-    $edit[$block['module'] . '_' . $block['delta'] . '[region]'] = 'left';
-    $this->drupalPost('admin/build/block', $edit, t('Save blocks'));
-
-    // Confirm that the block was moved to the proper region.
-    // TODO: Implement full region checking.
-    $this->assertText(t('The block settings have been updated.'), t('Block successfully moved to left region.'));
-
-    // Confirm that the block is being displayed.
-    $this->assertText(t($block['title']), t('Block successfully being displayed on the page.'));
+    // Check if the block can be moved to all availble regions.
+    foreach ($this->regions as $region) {
+      $this->moveBlockToRegion($block, $region);
+    }
 
     // Set the block to the disabled region.
     $edit = array();
@@ -115,16 +113,41 @@ class BlockTestCase extends DrupalWebTestCase {
     $this->assertText(t('The block settings have been updated.'), t('Block successfully move to disabled region.'));
     $this->assertNoText(t($block['title']), t('Block no longer appears on page.'));
 
+    // Confirm that the regions xpath is not availble
+    $xpath = '//div[@id="block-block-' . $bid . '"]/*';
+    $this->assertNoFieldByXPath($xpath, FALSE, t('Box found in no regions. '));
+
     // For convenience of developers, put the navigation block back.
     $edit = array();
     $edit[$block['module'] . '_' . $block['delta'] . '[region]'] = 'left';
     $this->drupalPost('admin/build/block', $edit, t('Save blocks'));
-    $this->assertText(t('The block settings have been updated.'), t('Block successfully move to disabled region.'));
+    $this->assertText(t('The block settings have been updated.'), t('Block successfully move to left region.'));
 
     $this->drupalPost('admin/build/block/configure/' . $block['module'] . '/' . $block['delta'], array('title' => 'Navigation'), t('Save block'));
     $this->assertText(t('The block configuration has been saved.'), t('Block title set.'));
   }
 
+  function moveBlockToRegion($block, $region) {
+    // If an id for an region hasn't been specified, we assume it's the same as the name.
+    if (!(isset($region['id']))) {
+      $region['id'] = $region['name'];
+    }
+
+    // Set the created block to a specific region.
+    $edit = array();
+    $edit[$block['module'] . '_' . $block['delta'] . '[region]'] = $region['name'];
+    $this->drupalPost('admin/build/block', $edit, t('Save blocks'));
+
+    // Confirm that the block was moved to the proper region.
+    $this->assertText(t('The block settings have been updated.'), t('Block successfully moved to %region_name region.', array( '%region_name'=> $region['name'])));
+
+    // Confirm that the block is being displayed.
+    $this->assertText(t($block['title']), t('Block successfully being displayed on the page.'));
+
+    // Confirm that the box was found at the proper region.
+    $xpath = '//div[@id="' . $region['id'] . '"]//div[@id="block-' . $block['module'] . '-' . $block['delta'] . '"]/*';
+    $this->assertFieldByXPath($xpath, FALSE, t('Box found in %region_name region.', array('%region_name' => $region['name'])));
+  }
 }
 
 class NonDefaultBlockAdmin extends DrupalWebTestCase {