A not-yet-often seen bug (or at least visual ugliness). Every multistep form is built twice. Thus, where the form building process calls drupal_set_message() (such as the #after_build element in the node form) this will cause each message to display twice.

Two possible fixes:

1) for 5.x: the form-building code should save and then restore the messages for one of the two instances of form building.

2) for 6.x: every unique message gets a unique ID (an additional parameter to drupal_set_message), like the form ID, or mail ID in drupal_mail. Thus, setting the same message many times results in only one message being output.

CommentFileSizeAuthor
#6 unique_message_50_1.diff599 bytespwolanin

Comments

pwolanin’s picture

pwolanin commented

Or, I just had a another idea- that might be good for 4.7.x forward: why not use the md5() of the message as the array key?

function drupal_set_message($message = NULL, $type = 'status') {
  if ($message) {
    if (!isset($_SESSION['messages'])) {
      $_SESSION['messages'] = array();
    }

    if (!isset($_SESSION['messages'][$type])) {
      $_SESSION['messages'][$type] = array();
    }
    // Insure that each unique message is only shown once.
    $key = md5($message);
    $_SESSION['messages'][$type][$key] = $message;
  }

  // messages not set when DB connection fails
  return isset($_SESSION['messages']) ? $_SESSION['messages'] : NULL;
}
RobRoy’s picture

RobRoy commented

I feel like we should fix the underlying problem that is resulting in a double call instead of "fixing" dsm().

eaton’s picture

eaton commented

This is relatively esoteric: drupal_set_message() is being called in an #after_build, which means that multistep forms pop up doubled messages. Really, we shouldn't be setting messages in forms at all: it causes weird display issues in Programmatically submitted forms, for example. That's why form_set_error() is used for logging errors, and *it* outputs drupal_set_message() calls when appropriate.

I'm not sure what the best short-term solution is, but I'll take a look at the patch.

chx’s picture

chx commented
Status: Active » Needs work

Even if decide that we want to unique every message, md5()'ing is totally unneeded, there is no size constrain on array keys. And arrays are hashes themselves, no need to manually hash.

pwolanin’s picture

pwolanin commented
Title: Multistep form handling of messages » handle mutiple identical messages

If I correctly grok what chx is suggesting, it would be:

$_SESSION['messages'][$type][$message] = $message;

I don't think it's actually that esoteric - I've seen problems with other modules that call drupal_set_message() in a place in the code that it may get called twice per page load. While I agree that this should be avoided, a simple change like this to insure each message only prints once seems like an easy one. Real patch to follow in a few hours.

pwolanin’s picture

pwolanin commented
Title: handle mutiple identical messages » handle mutiple identical messages in drupal_set_message()
Status: Needs work » Needs review
StatusFileSize
new unique_message_50_1.diff599 bytes

patch attached for 5.x. Tested and does result in multiple identical messages only being displayed once.

dries’s picture

dries commented
Status: Needs review » Closed (won't fix)

This is a feature, not a bug. The problem is not with drupal_set_message() but with the code calling drupal_set_message().

pwolanin’s picture

pwolanin commented
Title: handle mutiple identical messages in drupal_set_message() » multistep node form results in mutiple drupal_set_message() calls
Status: Closed (won't fix) » Active

Ok, if its a desired feature to show every message, then this isssue should just be about how to avoid the multiple calls to drupal_set_message(). I think the only way to do that is to eliminate the call to drupal_set_message as part of the #after_build step. Perhaps the message could just be generated as normal markup and pre-pended to the node preview?

RobRoy’s picture

RobRoy commented

@pwolanin - That's correct. Switch any multistep dsm() calls to #prefix markup and you'll be fine. Is this set to 'active' for a reason? Does this problem exist in core somewhere?

pwolanin’s picture

pwolanin commented

Yes, the problem exists in the node form-

  1. The node preview is built through an #after_build elements that adds content to the #prefix element.
  2. The node preview may call drupal_set_message to provide the message about using the <break> tag.
  3. The core poll module defines the node form as multistep (and I assume other content types may in the future).
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Closed (won't fix)

This is won't fix per #7. If #10 is still an issue, please open a new issue.