When update a node, without creating a new revision, entering a revision log of "0" is ignored. This is because of code:
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.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | node-revision-log-1282956-5007962.patch | 580 bytes | pgrond |
| #8 | node-revision-log-1282956-5006222.patch | 581 bytes | rickmanelius |
| #5 | node-revision-log-1282956-5005882.patch | 581 bytes | rickmanelius |
Comments
Comment #1
Everett Zufelt commentedThis 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.
Comment #2
Everett Zufelt commentedWhoops,
realized that it is empty() and not !empty(), so comment #1 is wrong.
Can you please point to the function where this occurs?
Comment #3
lyricnz commentedYes, 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
Comment #4
rickmanelius commentedI believe the line is solved by converting it to:
It's also possible with the following, but I think this may be more confusing
Patch coming soon...
Comment #5
rickmanelius commentedPatch attached
Comment #6
rickmanelius commentedComment #7
lyricnz commentedThat condition would probably be clearer if it was:
i.e. "if ($node->log is not set, or it's literally an empty string) then ..."
Comment #8
rickmanelius commentedResubmit with suggestions in #7
Comment #10
edb commentedYou have an extra bracket, should be:
Comment #11
pgrond commentedResubmit with fix from #10
Comment #12
rickmanelius commentedThanks edb. I was moving too quickly on such a simple thing. Thanks to pgrond for the new patch.
Comment #14
rickmanelius commentedThanks 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.
Comment #15
lyricnz commentedBackport would be useful, at least to D7
Comment #16
rickmanelius commentedSure. 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?
Comment #17
lyricnz commentedPatch will probably apply to D7, and maybe D6 without modification. Let's get it in D8 first.
Comment #18
catchI 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.
Comment #19
webchickSeems a bit silly, but I guess it doesn't hurt anything.
Committed and pushed to 7.x. Thanks!
Comment #20
lyricnz commentedOh, 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.