When update a node, without creating a new revision, entering a revision log of "0" is ignored. This is because of code:

<?php
elseif (empty($node->log))
?>

in node_save() - PHP treats "0" as empty. This bug has been present since at least 6.x, and there are presumably lots of other instances of the same issue.

Files: 
CommentFileSizeAuthor
#11 node-revision-log-1282956-5007962.patch580 bytespgrond
PASSED: [[SimpleTest]]: [MySQL] 33,003 pass(es).
[ View ]
#8 node-revision-log-1282956-5006222.patch581 bytesrickmanelius
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in modules/node/node.module.
[ View ]
#5 node-revision-log-1282956-5005882.patch581 bytesrickmanelius
PASSED: [[SimpleTest]]: [MySQL] 32,980 pass(es).
[ View ]

Comments

Issue tags:+Novice

This is the third related empty causes '0' to fail issue I've run across recently. I believe the others were for radio / checkbox #title and fieldset #title (legend).

The correct code should be:

elseif (isset($node->log) && $node->log !== '')

Presuming that isset($node->log) isn't tested / verfied somewhere earlier in the logic.

Whoops,

realized that it is empty() and not !empty(), so comment #1 is wrong.

Can you please point to the function where this occurs?

Yes, it's a very common bug (I did a quick check now, found several in common.inc alone - will create a few more issues until I get bored)

Bug is in node_save() which is in modules/node/node.module - around line 1076

I believe the line is solved by converting it to:

elseif (!(isset($node->log) && $node->log != '')) {

It's also possible with the following, but I think this may be more confusing

elseif (empty($node->log) && $node->log != 0.0) {

Patch coming soon...

StatusFileSize
new581 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,980 pass(es).
[ View ]

Patch attached

Status:Active» Needs review

Status:Needs review» Needs work

That condition would probably be clearer if it was:

<?php
   
elseif (!isset($node->log) || $node->log === '')) {
?>

i.e. "if ($node->log is not set, or it's literally an empty string) then ..."

Status:Needs work» Needs review
StatusFileSize
new581 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in modules/node/node.module.
[ View ]

Resubmit with suggestions in #7

Status:Needs review» Needs work

The last submitted patch, node-revision-log-1282956-5006222.patch, failed testing.

You have an extra bracket, should be:

<?php
   
elseif (!isset($node->log) || $node->log === '') {
?>

Status:Needs work» Needs review
StatusFileSize
new580 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,003 pass(es).
[ View ]

Resubmit with fix from #10

Thanks edb. I was moving too quickly on such a simple thing. Thanks to pgrond for the new patch.

Status:Needs review» Reviewed & tested by the community

Thanks for the syntax fix in #11. Tested and it works. Logs of "0" and "0.0" (without creating a new revision) now save. Reviewed and tested.

Backport would be useful, at least to D7

Sure. What's the protocol for including backport patches in the same issue? The patch above says nothing of being D8 specific except the tag in this issue. Do we roll the patch, change the status, and then attach?

Patch will probably apply to D7, and maybe D6 without modification. Let's get it in D8 first.

Version:8.x-dev» 7.x-dev

I can't think of a reason you'd want to add a revision with 0 (so am not asking for a test here), but can't think of a reason to prevent that either.

Moving back to D7 for webchick.

Status:Reviewed & tested by the community» Fixed

Seems a bit silly, but I guess it doesn't hurt anything.

Committed and pushed to 7.x. Thanks!

Oh, totally silly! After fixing an empty() bug in my code, I checked a couple of places in D7 for the same bug. Yup, lots of 'em. Meh.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.