Index: mollom.admin.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/mollom/mollom.admin.inc,v
retrieving revision 1.1.2.34
diff -u -p -r1.1.2.34 mollom.admin.inc
--- mollom.admin.inc	3 Sep 2010 18:53:51 -0000	1.1.2.34
+++ mollom.admin.inc	12 Sep 2010 16:22:29 -0000
@@ -445,63 +445,66 @@ function mollom_admin_blacklist_delete_s
 
 /**
  * Form builder; Global Mollom settings form.
+ *
+ * This form does not validate Mollom API keys, since the fallback method still
+ * needs to be able to be reconfigured in case Mollom services are down.
+ * mollom.verifyKey would invalidate the keys and throw an error; hence,
+ * _mollom_fallback() would invoke form_set_error(), effectively preventing this
+ * form from submitting.
  */
 function mollom_admin_settings(&$form_state) {
-  // When a user visits the Mollom administration page, automatically verify the
-  // keys and output any error messages.
+  // Output a positive status message, since users keep on asking whether
+  // Mollom should work or not. Re-check on every regular visit of this form to
+  // verify the module's configuration.
   if (empty($form_state['post'])) {
-    $status = _mollom_status(TRUE);
+    $status = _mollom_status();
+    // If there is any configuration error, then mollom_init() will have output
+    // it already.
     if ($status === TRUE) {
-      // Output a positive status message, since users keep on asking whether
-      // Mollom should work or not.
       drupal_set_message(t('We contacted the Mollom servers to verify your keys: the Mollom services are operating correctly. We are now blocking spam.'));
     }
-    elseif ($status['keys valid'] === NETWORK_ERROR) {
-      drupal_set_message(t('We tried to contact the Mollom servers but we encountered a network error. Please make sure that your web server can make outgoing HTTP requests.'), 'error');
-    }
-    elseif ($status['keys valid'] === MOLLOM_ERROR) {
-      drupal_set_message(t('We contacted the Mollom servers to verify your keys: your keys do not exist or are no longer valid. Please visit the <em>Manage sites</em> page on the Mollom website again: <a href="@mollom-user">@mollom-user</a>.', array('@mollom-user' => 'http://mollom.com/user')), 'error');
-    }
   }
 
-  $form['server'] = array(
-    '#type' => 'fieldset',
-    '#title' => t('Fallback strategy'),
-    '#description' => t('When the Mollom servers are down or otherwise unreachable, no text analysis is performed and no CAPTCHAs are generated. If this occurs, your site will use the configured fallback strategy. Subscribers to <a href="@pricing-url">Mollom Plus</a> receive access to <a href="@sla-url">Mollom\'s high-availability backend infrastructure</a>, not available to free users, reducing potential downtime.', array(
-      '@pricing-url' => 'http://mollom.com/pricing',
-      '@sla-url' => 'http://mollom.com/standard-service-level-agreement',
-    )),
-  );
-  $form['server']['mollom_fallback'] = array(
-    '#type' => 'radios',
-    // Default to treating everything as inappropriate.
-    '#default_value' => variable_get('mollom_fallback', MOLLOM_FALLBACK_BLOCK),
-    '#options' => array(
-      MOLLOM_FALLBACK_BLOCK => t('Block all submissions of protected forms until the server problems are resolved'),
-      MOLLOM_FALLBACK_ACCEPT => t('Leave all forms unprotected and accept all submissions'),
-    ),
-  );
-
   $form['access-keys'] = array(
     '#type' => 'fieldset',
-    '#title' => t('Mollom access keys'),
+    '#title' => t('Access keys'),
     '#description' => t('To use Mollom, you need a public and private key. To obtain your keys, <a href="@mollom-login-url">register and login on mollom.com</a>, and <a href="@mollom-manager-add-url">create a subscription</a> for your site. Once you created a subscription, copy your private and public access keys from the <a href="@mollom-manager-url">site manager</a> into the form fields below, and you are ready to go.', array(
       '@mollom-login-url' => 'http://mollom.com/user',
       '@mollom-manager-add-url' => 'http://mollom.com/site-manager/add',
       '@mollom-manager-url' => 'http://mollom.com/site-manager',
     )),
+    '#collapsible' => TRUE,
+    // Only show key configuration fields if they are not configured or invalid.
+    '#collapsed' => !isset($status) ? FALSE : $status === TRUE,
   );
+  // Keys are not #required to allow to install this module and configure it
+  // later.
   $form['access-keys']['mollom_public_key'] = array(
     '#type' => 'textfield',
     '#title' => t('Public key'),
     '#default_value' => variable_get('mollom_public_key', ''),
-    '#description' => t('The public key is used to uniquely identify you.'),
+    '#description' => t('Used to uniquely identify you.'),
   );
   $form['access-keys']['mollom_private_key'] = array(
     '#type' => 'textfield',
     '#title' => t('Private key'),
     '#default_value' => variable_get('mollom_private_key', ''),
-    '#description' => t('The private key is used to prevent someone from hijacking your requests. Similar to a password, it should never be shared with anyone.'),
+    '#description' => t('Used to prevent someone else from hijacking your requests. Similar to a password, it should never be shared with anyone.'),
+  );
+
+  $form['mollom_fallback'] = array(
+    '#type' => 'radios',
+    '#title' => t('Fallback strategy for protected forms'),
+    // Default to treating everything as inappropriate.
+    '#default_value' => variable_get('mollom_fallback', MOLLOM_FALLBACK_BLOCK),
+    '#options' => array(
+      MOLLOM_FALLBACK_BLOCK => t('Block all form submissions'),
+      MOLLOM_FALLBACK_ACCEPT => t('Accept all form submissions'),
+    ),
+    '#description' => t('In case the Mollom services are unreachable, no text analysis is performed and no CAPTCHAs are generated. If this occurs, your site will use the configured fallback strategy until the server problems are resolved. Subscribers to <a href="@pricing-url">Mollom Plus</a> receive access to <a href="@sla-url">Mollom\'s high-availability backend infrastructure</a>, not available to free users, reducing potential downtime.', array(
+      '@pricing-url' => 'http://mollom.com/pricing',
+      '@sla-url' => 'http://mollom.com/standard-service-level-agreement',
+    )),
   );
 
   $form['mollom_privacy_link'] = array(
Index: mollom.install
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/mollom/mollom.install,v
retrieving revision 1.2.2.31
diff -u -p -r1.2.2.31 mollom.install
--- mollom.install	6 Apr 2010 22:17:51 -0000	1.2.2.31
+++ mollom.install	12 Sep 2010 16:33:28 -0000
@@ -8,11 +8,26 @@
 
 /**
  * Implements hook_requirements().
+ *
+ * @param $check
+ *   (optional) Boolean whether to re-check the module's installation and
+ *   configuration status. Defaults to TRUE, as this argument is not passed for
+ *   hook_requirements() by default. Passing FALSE allows other run-time code
+ *   to re-generate requirements error messages to be displayed on other pages
+ *   than the site's system status report page.
+ *
+ * @see mollom_init()
+ * @see mollom_admin_settings()
+ * @see _mollom_status()
  */
-function mollom_requirements($phase = 'runtime') {
+function mollom_requirements($phase = 'runtime', $check = TRUE) {
   $requirements = array();
   if ($phase == 'runtime') {
-    $status = _mollom_status(TRUE);
+    // This is invoked from both mollom_install() and mollom_init(); make sure
+    // that mollom.module is loaded.
+    drupal_load('module', 'mollom');
+
+    $status = _mollom_status($check);
     // Immediately return if everything is in order.
     if ($status === TRUE) {
       return $requirements;
@@ -24,19 +39,35 @@ function mollom_requirements($phase = 'r
       'value' => '',
       'severity' => REQUIREMENT_ERROR,
     );
+    // Append a link to the settings page to the error message on all pages,
+    // except on the settings page itself. These error messages also need to be
+    // shown on the settings page, since Mollom API keys can be entered later.
+    $admin_message = '';
+    if ($_GET['q'] != 'admin/settings/mollom/settings') {
+      $admin_message = t('Visit the <a href="@settings-url">Mollom settings page</a> to configure your keys.', array(
+        '@settings-url' => url('admin/settings/mollom/settings'),
+      ));
+    }
+    // Generate an appropriate error message:
     // Missing API keys.
     if (!$status['keys']) {
       $requirements['mollom']['value'] = t('Not configured');
-      $requirements['mollom']['description'] = t('Mollom API keys are not <a href="@settings-url">configured</a> yet.', array('@settings-url' => url('admin/settings/mollom/settings')));
+      $requirements['mollom']['description'] = t('The Mollom API keys are not configured yet. !admin-message', array(
+        '!admin-message' => $admin_message,
+      ));
+    }
+    // Invalid API keys.
+    elseif ($status['keys valid'] === MOLLOM_ERROR) {
+      $requirements['mollom']['value'] = t('Invalid');
+      $requirements['mollom']['description'] = t('The configured Mollom API keys are invalid. !admin-message', array(
+        '!admin-message' => $admin_message,
+      ));
     }
+    // Communication error.
     elseif ($status['keys valid'] === NETWORK_ERROR) {
       $requirements['mollom']['value'] = t('Network error');
       $requirements['mollom']['description'] = t('The Mollom servers could not be contacted. Please make sure that your web server can make outgoing HTTP requests.');
     }
-    elseif ($status['keys valid'] === MOLLOM_ERROR) {
-      $requirements['mollom']['value'] = t('Invalid');
-      $requirements['mollom']['description'] = t('The <a href="@settings-url">configured</a> Mollom API keys are invalid.', array('@settings-url' => url('admin/settings/mollom/settings')));
-    }
   }
   return $requirements;
 }
@@ -145,6 +176,15 @@ function mollom_schema() {
  */
 function mollom_install() {
   drupal_install_schema('mollom');
+
+  // Point the user to Mollom's settings page after installation.
+  $requirements = mollom_requirements('runtime', FALSE);
+  // When running tests in D6, hook_install() seems to be invoked very (too?)
+  // early, leading to a potentially predefined valid module configuration from
+  // the parent site, therefore throwing a PHP notice here.
+  if (isset($requirements['mollom'])) {
+    drupal_set_message($requirements['mollom']['description'], 'warning');
+  }
 }
 
 /**
Index: mollom.module
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/mollom/mollom.module,v
retrieving revision 1.2.2.159
diff -u -p -r1.2.2.159 mollom.module
--- mollom.module	11 Sep 2010 01:04:38 -0000	1.2.2.159
+++ mollom.module	12 Sep 2010 16:26:18 -0000
@@ -154,6 +154,27 @@ function mollom_help($path, $arg) {
 }
 
 /**
+ * Implements hook_init().
+ */
+function mollom_init() {
+  // On all Mollom administration pages, check the module configuration and
+  // display the corresponding requirements error, if invalid.
+  if (empty($_POST) && strpos($_GET['q'], 'admin/settings/mollom') === 0 && user_access('administer mollom')) {
+    // Re-check the status on the settings form only.
+    $status = _mollom_status($_GET['q'] == 'admin/settings/mollom/settings');
+    if ($status !== TRUE) {
+      // Fetch and display requirements error message, without re-checking.
+      module_load_install('mollom');
+      $requirements = mollom_requirements('runtime', FALSE);
+      drupal_set_message($requirements['mollom']['description'], 'error');
+    }
+  }
+  if (strpos($_GET['q'], 'admin/reports/event/') === 0) {
+    drupal_add_css(drupal_get_path('module', 'mollom') . '/mollom.css');
+  }
+}
+
+/**
  * Implements hook_link().
  */
 function mollom_link($type, $object, $teaser = FALSE) {
@@ -341,21 +362,6 @@ function mollom_report_access($entity, $
 }
 
 /**
- * Implements hook_init().
- */
-function mollom_init() {
-  if (empty($_POST) && variable_get('mollom_testing_mode', 0) && user_access('administer mollom')) {
-    $message = t('Mollom testing mode is still enabled. Visit the <a href="@settings-url">Mollom settings page</a> to disable developer mode.', array(
-      '@settings-url' => url('admin/settings/mollom/settings'),
-    ));
-    drupal_set_message($message, 'warning');
-  }
-  if (strpos($_GET['q'], 'admin/reports/event/') === 0) {
-    drupal_add_css(drupal_get_path('module', 'mollom') . '/mollom.css');
-  }
-}
-
-/**
  * Implements hook_perm().
  */
 function mollom_perm() {
@@ -575,6 +581,20 @@ function mollom_form_alter(&$form, &$for
     return;
   }
 
+  // @todo Show this message on all protected forms, regardless of permissions.
+  if (empty($_POST) && variable_get('mollom_testing_mode', 0)) {
+    $admin_message = '';
+    if (user_access('administer mollom') && $_GET['q'] != 'admin/settings/mollom/settings') {
+      $admin_message = t('Visit the <a href="@settings-url">Mollom settings page</a> to disable it.', array(
+        '@settings-url' => url('admin/settings/mollom/settings'),
+      ));
+    }
+    $message = t('Mollom testing mode is still enabled. !admin-message', array(
+      '!admin-message' => $admin_message,
+    ));
+    drupal_set_message($message, 'warning');
+  }
+
   // Site administrators don't have their content checked with Mollom.
   if (!user_access('bypass mollom protection')) {
     // Retrieve a list of all protected forms once.
@@ -1073,6 +1093,7 @@ function _mollom_get_openid($account) {
 function _mollom_status($reset = FALSE) {
   // Load stored status.
   $status = variable_get('mollom_status', array(
+    'keys' => FALSE,
     'keys valid' => FALSE,
   ));
 
@@ -1084,18 +1105,20 @@ function _mollom_status($reset = FALSE) 
   // If we have keys and are asked to reset, check whether keys are valid.
   if ($status['keys'] && $reset) {
     $status['keys valid'] = mollom('mollom.verifyKey', _mollom_get_version());
-    variable_set('mollom_status', $status);
   }
 
-  if ($status['keys valid'] === TRUE) {
-    return TRUE;
+  // In case of an error, indicate whether we have a non-empty server list.
+  if ($status['keys valid'] !== TRUE) {
+    $servers = variable_get('mollom_servers', array());
+    $status['servers'] = !empty($servers);
   }
 
-  // In case of an error, also indicate whether we have a non-empty server list.
-  $servers = variable_get('mollom_servers', array());
-  $status['servers'] = !empty($servers);
+  // Update stored status upon reset.
+  if ($reset) {
+    variable_set('mollom_status', $status);
+  }
 
-  return $status;
+  return ($status['keys valid'] === TRUE ? TRUE : $status);
 }
 
 /**
@@ -1103,6 +1126,10 @@ function _mollom_status($reset = FALSE) 
  */
 function _mollom_fallback() {
   $fallback = variable_get('mollom_fallback', MOLLOM_FALLBACK_BLOCK);
+  // @todo Prevents mollom_admin_settings() from implementing a proper form
+  //   validation. Add !empty($_POST) to this condition + manually invoke from
+  //   mollom_process_form() on GET requests? Or don't call it from mollom()?
+  //   Anything, but just don't mix FAPI logic into XML-RPC logic.
   if ($fallback == MOLLOM_FALLBACK_BLOCK) {
     form_set_error('mollom', t("The spam filter installed on this site is currently unavailable. Per site policy, we are unable to accept new submissions until that problem is resolved. Please try resubmitting the form in a couple of minutes."));
   }
Index: tests/mollom.test
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/mollom/tests/mollom.test,v
retrieving revision 1.1.2.56
diff -u -p -r1.1.2.56 mollom.test
--- tests/mollom.test	9 Sep 2010 15:11:16 -0000	1.1.2.56
+++ tests/mollom.test	12 Sep 2010 16:22:29 -0000
@@ -663,11 +663,11 @@ class MollomWebTestCase extends DrupalWe
 /**
  * Tests module installation and global status handling.
  */
-class MollomStatusTestCase extends MollomWebTestCase {
+class MollomInstallationTestCase extends MollomWebTestCase {
   public static function getInfo() {
     return array(
-      'name' => 'Status handling',
-      'description' => 'Tests module installation and global status handling.',
+      'name' => 'Installation and key handling',
+      'description' => 'Tests module installation and key error handling.',
       'group' => 'Mollom',
     );
   }
@@ -690,40 +690,57 @@ class MollomStatusTestCase extends Mollo
 
   /**
    * Tests status handling after installation.
+   *
+   * We walk through a regular installation of the Mollom module instead of using
+   * setUp() to ensure that everything works as expected.
+   *
+   * Note: Partial error messages tested here; hence, no t().
    */
-  function testStatusInstallation() {
-    // Ensure there is no requirements error by default.
+  function testInstallationProcess() {
+    $admin_message = t('Visit the <a href="@settings-url">Mollom settings page</a> to configure your keys.', array(
+      '@settings-url' => url('admin/settings/mollom/settings'),
+    ));
     $this->drupalLogin($this->admin_user);
+
+    // Ensure there is no requirements error by default.
     $this->drupalGet('admin/reports/status');
     $this->clickLink('run cron manually');
 
-    // Install the module.
+    // Install the Mollom module.
     $this->drupalPost('admin/build/modules', array('status[mollom]' => TRUE), t('Save configuration'));
+    $this->assertRaw(t('The Mollom API keys are not configured yet. !admin-message', array(
+      '!admin-message' => $admin_message,
+    )), t('Post installation warning found.'));
 
-    // Verify that forms can be submitted without valid module configuration.
+    // Verify that forms can be submitted without valid Mollom module configuration.
     $node = $this->drupalCreateNode(array('type' => 'story', 'promoted' => TRUE));
     $this->drupalLogin($this->web_user);
     $this->drupalGet('comment/reply/' . $node->nid);
     $edit = array(
-      'comment' => $this->randomName(),
+      'comment' => 'spam',
     );
     $this->drupalPost(NULL, $edit, t('Preview'));
     $this->drupalPost(NULL, array(), t('Save'));
     $this->assertRaw('<p>' . $edit['comment'] . '</p>', t('Comment found.'));
 
-    // Verify requirements error about missing API keys.
+    // Assign the 'administer mollom' permission and log in a user.
     $this->drupalLogin($this->admin_user);
-    $this->drupalGet('admin/reports/status');
-    // @todo Should a user without Mollom administration access see a
-    //   requirement error containing a link to the settings page?
-    $this->assertRaw(t('Mollom API keys are not <a href="@settings-url">configured</a> yet.', array('@settings-url' => url('admin/settings/mollom/settings'))), t('Requirements error found.'));
-
-    // Grant access to Mollom settings.
     $edit = array(
       DRUPAL_AUTHENTICATED_RID . '[administer mollom]' => TRUE,
     );
     $this->drupalPost('admin/user/permissions', $edit, t('Save permissions'));
 
+    // Verify presence of 'empty keys' error message.
+    $this->drupalGet('admin/settings/mollom');
+    $this->assertText('The Mollom API keys are not configured yet.');
+    $this->assertNoText('The configured Mollom API keys are invalid.');
+
+    // Verify requirements error about missing API keys.
+    $this->drupalGet('admin/reports/status');
+    $this->assertRaw(t('The Mollom API keys are not configured yet. !admin-message', array(
+      '!admin-message' => $admin_message,
+    )), t('Requirements error found.'));
+
     // Configure invalid keys.
     $edit = array(
       'mollom_public_key' => 'foo',
@@ -732,20 +749,29 @@ class MollomStatusTestCase extends Mollo
     $this->drupalPost('admin/settings/mollom/settings', $edit, t('Save configuration'), array('watchdog' => FALSE));
     $this->assertText(t('The configuration options have been saved.'));
     $this->assertNoText($this->fallback_message, t('Fallback message not found.'));
-    $this->assertNoText(t('We tried to contact the Mollom servers but we encountered a network error. Please make sure that your web server can make outgoing HTTP requests.'), t('Network error message not found.'));
-    $this->assertRaw(t('We contacted the Mollom servers to verify your keys: your keys do not exist or are no longer valid. Please visit the <em>Manage sites</em> page on the Mollom website again: <a href="@mollom-user">@mollom-user</a>.', array('@mollom-user' => 'http://mollom.com/user')), t('Invalid keys error message found.'));
+
+    // Verify presence of 'incorrect keys' error message.
+    $this->assertText('The configured Mollom API keys are invalid.');
+    $this->assertNoText('The Mollom API keys are not configured yet.');
+    $this->assertNoText(t('The Mollom servers could not be contacted. Please make sure that your web server can make outgoing HTTP requests.'));
 
     // Verify requirements error about invalid API keys.
     $this->drupalGet('admin/reports/status', array('watchdog' => FALSE));
-    $this->assertRaw(t('The <a href="@settings-url">configured</a> Mollom API keys are invalid.', array('@settings-url' => url('admin/settings/mollom/settings'))), t('Requirements error found.'));
+    $this->assertText('The configured Mollom API keys are invalid.');
 
-    // Replace server list with unreachable servers.
+    // Ensure unreachable servers.
+    variable_set('mollom_servers', array('http://fake-host'));
+
+    // Verify presence of 'network error' message.
+    $this->drupalGet('admin/settings/mollom/settings', array('watchdog' => FALSE));
+    $this->assertText(t('The Mollom servers could not be contacted. Please make sure that your web server can make outgoing HTTP requests.'));
+
+    // Ensure unreachable servers.
     variable_set('mollom_servers', array('http://fake-host'));
 
     // Verify requirements error about network error.
-    // Go directly to the status report, since the server list will be reset.
     $this->drupalGet('admin/reports/status', array('watchdog' => FALSE));
-    $this->assertText(t('The Mollom servers could not be contacted. Please make sure that your web server can make outgoing HTTP requests.'), t('Requirements error found.'));
+    $this->assertText(t('The Mollom servers could not be contacted. Please make sure that your web server can make outgoing HTTP requests.'));
     $this->assertNoText($this->fallback_message, t('Fallback message not found.'));
 
     // Verify that valid keys work.
@@ -758,9 +784,11 @@ class MollomStatusTestCase extends Mollo
     $this->drupalPost(NULL, $edit, t('Save configuration'));
     $this->assertText(t('The configuration options have been saved.'));
     $this->assertText('We are now blocking spam.');
-    $this->assertNoText('your keys do not exist or are no longer valid.');
+    $this->assertNoText('The Mollom API keys are not configured yet.');
+    $this->assertNoText('The configured Mollom API keys are invalid.');
 
     // Verify presence of testing mode warning.
+    $this->drupalGet('admin/settings/mollom/blacklist');
     $this->assertText('Mollom testing mode is still enabled.');
   }
 }
@@ -886,7 +914,7 @@ class MollomAccessTestCase extends Mollo
     );
     $this->drupalPost(NULL, $edit, t('Save configuration'), array('watchdog' => FALSE));
     $this->assertText(t('The configuration options have been saved.'));
-    $this->assertRaw(t('We contacted the Mollom servers to verify your keys: your keys do not exist or are no longer valid. Please visit the <em>Manage sites</em> page on the Mollom website again: <a href="@mollom-user">@mollom-user</a>.', array('@mollom-user' => 'http://mollom.com/user')), t('Invalid keys error message appears.'));
+    $this->assertText('The configured Mollom API keys are invalid.');
   }
 
   /**
