Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Obviously the string '0' is not a particularly helpful message and so maybe in real world use cases it would not be done but sometimes I use drupal_set_message on my development server as a quick debugging tool. Setting the message to be the values of variables, I expect to see all the values output in the same way. But if the value passed is a numerical or string zero, then no message at all (not even an empty < li >< /li > pair) is output. This seems wrong!
Sorry that I don't know which component bootstrap.inc belongs to.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | after.PNG | 2.14 KB | dcam |
| #21 | D7-1845104-21-drupal_set_message-zero.patch | 551 bytes | er.pushpinderrana |
Comments
Comment #1
petebarnett CreditAttribution: petebarnett commentedFrom reading the docs, I'd say the intended purpose of
drupal_set_message(...)is to store user targeted (admin or otherwise) natural language messages. The docs sayAdditional contrib tools are available for debugging and displaying PHP primitives in the form of the Devel module, with its
dpm()etc.For that reason, I don't believe this can be considered a bug. On the contrary, if any module was outputting the rather unhelpful message of '0', I would consider that a bug. This case only arises from the undesigned use of the function as a developer tool.
If you're interested, the reason that you can't display '0' is that
drupal_set_message()on line 1757 of bootstrap.inc evaluates$messageitself, and only proceeds to store the message if that string evaluates to TRUE. In PHP '0' will evaluate to FALSE.Comment #2
martin_qYou're right of course, and I was attempting to acknowledge this in my initial post. Whilst '0' shouldn't ever be a valid message, for the reasons you give, the purist in me says that
if ($message)in line 1757 of bootstrap.inc is sloppy and usingif(isset($message))would be better practice!Hmmm, I guess the idea is to avoid printing a message with an empty string (""). I still haven't accepted the idea that "0" should also be considered an empty string. Maybe my beef is with PHP, not Drupal... :)
Comment #3
thehong CreditAttribution: thehong commentedComment #4
martin_qWorks for me. As the OP, am I allowed to set it to 'reviewed and tested by the community'?
Comment #5
longwaveAs trivial as this is, it should be fixed in 8.x first and then backported.
Comment #6
longwave#3: 1845104.diff queued for re-testing.
Comment #8
martin_qThe failure is, as far as I can tell, because bootstrap.inc has moved from includes/ to core/includes/. The line number for the change is now 1801. I am not currently set up with git or D8 though so I can't roll a new diff. I imagine just hand-modifying the file here would be of little use for merging the change in etc, so I won't do that.
Comment #9
longwaveNot sure if this needs tests; there don't seem to be any other explicit tests for drupal_set_message().
Comment #9.0
longwaveAdded spaces to prevent html tags being taken literally.
Comment #11
marabak CreditAttribution: marabak commentedthe patch #9 is working fine.
Comment #12
alexpottCommitted 74857b5 and pushed to 8.0.x. Thanks!
Comment #14
longwave#3 could still be committed to 7.x, if anyone really cares.
Comment #16
longwave#3 is RTBC.
Comment #18
dcam CreditAttribution: dcam commentedRe-uploading #3 so Testbot quits re-testing the D8 patch. I am not the author and it should not be attributed to me. Setting this back to RTBC per #16.
Comment #19
David_Rothstein CreditAttribution: David_Rothstein commentedI did a quick test of drupal_set_message('') with this patch and it looked really messed up on a couple different themes.
At least for Drupal 7 perhaps we should continue to not print anything in that case (or drupal_set_message(FALSE) too?) to avoid any such hidden bugs in people's code popping up on real sites...?
So maybe something like
if ($message || $message === '0')...Comment #20
David_Rothstein CreditAttribution: David_Rothstein commentedOr maybe
if ($message || $message === '0' || $message === 0)to be really robust...Comment #21
er.pushpinderrana CreditAttribution: er.pushpinderrana commentedIncorporated #20 recommendation in this patch.
Comment #24
dcam CreditAttribution: dcam commentedThat was a random test failure.
Comment #25
dcam CreditAttribution: dcam commentedWith the patch applied
'0'and0can be passed as the message parameter in drupal_set_message().Comment #26
rukayya CreditAttribution: rukayya commentedthe patch #9 is worked for me
Comment #27
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedCommitted to 7.x - thanks!
Adding this to the release notes since it is a slight behavior change in drupal_set_message(), in particular with regard to whether the function returns messages or not.
Comment #29
Studiographene CreditAttribution: Studiographene as a volunteer and commentedthe patch #9 is worked for me
Thanks