what i did

i've got a fresh copy of drupal from the cvs head (as of 2006-03-08, 3am GMT-800) running on a test site. i've been playing with node revision behavior (trying, unsuccessfully, to reproduce the Edit node as new revision gives mysql error bug), and i stumbled upon what appear to be a number of problems with the "Create new revision" publishing option.

what i saw

  • i went to admin/settings/content-types/page and selected the "Create new revision" checkbox, and saved. once i went to node/add/page, the initial version of the node has the "Create new revision" box checked, and a "Log message" textarea. i made a test page, put in a log message, and hit save. however, when i then try to edit the node, the "Create new revision" checkbox is now unset, yet the "Log message" area is still there.
  • if i manually select the "Create new revision" box while editing the node, i get a new revision created, and a revisions tab appears (including the previous value of the "Log message" field as the old revision, and the current value as the new revision. this much is working. however, again, when i go to edit the page, the "Create new revision" checkbox is not checked. if i make further changes (e.g. to the Log message field and/or the node body), that shows up as the current revision, not as a new revision.

what i'd expect

  • once you select the site-wide "create new revision" box, that should be remembered for any nodes of that type, even beyond the first revision.
  • the "Log message" textarea should always appear if you're creating a new revision, and should never appear if you're not.

those items match 4.6 behavior, as best as i can tell. i think 4.7 should at least do what 4.6 is doing.

furthermore, i think these would be improvements:

  • even if the site-wide setting isn't set, once you select the "create new revision" box for a particular node, that should be remembered and future edits should have "create new revision" selected already. if you want to make a simple change that you don't want to track, you can go out of your way to uncheck the box, but i think once you say you want to save revisions of a node, that should be the default behavior for all edits of that node.
  • it's slightly confusing that the first revision is considered a special case and you don't get a revisions tab unless there are multiple revisions. in 4.6 the revisions tab only showed you "older revisions", whereas in 4.7, this tab shows the current revision (and the log messages, nice!). if 4.7 is going to consider the 1st revision a valid revision, why not have the tab? "because the tab doesn't let you do anything unless there are N revisions" you might reply. however, i'd argue the existance of the tab, even on the first revision, is a nice reassurance to the user that drupal really is planning to keep track of revisions for you, even if there are no older revisions to view or revert to just yet.

it's too late for me to wrap my head around this problem and submit a patch. i just wanted to create an issue before i forgot about it. ;) i tried to search the existing issue queue but didn't see anything about this. apologies if this is duplicate. i'm setting the status to "critical" because it appears that the site-wide setting isn't doing what it's supposed to, 4.7 is a regression from 4.6 in terms of node revision behavior, and i think that's going to look bad for drupal, especially folks trying to upgrade who are used to the old behavior. apologies if those aren't good enough reasons for this to be "critical". ;) obviously, my "improvements" ideas aren't part of the "critical bug" i have in mind, they're just related from a UI perspective, and i thought i should provide a little more context.

thanks,
-derek

CommentFileSizeAuthor
#9 revisions2.patch1.44 KBZen
#5 revisions_19.patch1.35 KBZen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

Gerhard, do you have time to focus on the revision related problems?

killes@www.drop.org’s picture

So we'd need to fix this tow things?

* once you select the site-wide "create new revision" box, that should be remembered for any nodes of that type, even beyond the first revision.
* the "Log message" textarea should always appear if you're creating a new revision, and should never appear if you're not.

I have a suspicion why the first thing fails: All the other publishing options are indeed per node and not only per node type and thus they get loaded when you do node_load.

The second thing is difficult to do. We cannot know if you whish to select the "create new revision" button or not.

puregin’s picture

Re: point two: perhaps the log message textarea could be displayed as part of validation if a revision is being saved, on a second form. This would mean one more step in saving a node, but it would also simplify the node edit form.

dww’s picture

Re: point two: perhaps the log message textarea could be displayed as part of validation if a revision is being saved, on a second form. This would mean one more step in saving a node, but it would also simplify the node edit form.

+1, if it's not too complicated to implement. it'd be a lot more like using traditional revision control: you make your changes, and then once you "commit" (preview/save in our case), you're asked for your log message.

Zen’s picture

Assigned: Unassigned » Zen
Priority: Critical » Normal
Status: Active » Needs review
FileSize
1.35 KB

Downgrading as this isn't critical.

Attached patch:

  • Makes the revision checkbox use the "default" value (as indicated by content type settings) regardless of whether the node is being updated or created.
  • Simplifies a section of related code.

I would personally prefer to see the log message available for all node types that have the option of having revisions. I don't see how one can exist without the other.

Please review,
Thanks
-K

killes@www.drop.org’s picture

looks good to me, did you test it?

Zen’s picture

I did so, yes.

-K

dww’s picture

Priority: Normal » Critical

i've tested as well, and it definitely fixes this part: "once you select the site-wide "create new revision" box, that should be remembered for any nodes of that type, even beyond the first revision.". i agree, once this part is committed, this issue is no longer critical. however, until revisions_19.patch is committed, i think it's still critical (and both dries and killes commented on this issue without changing the status) so i'm setting it back.

after that's fixed, i still think the log message textarea needs more thought. right now, if an anonymous user has permissions to create a certain type of node where the site-wide default is to not save revisions, this user has no way to create/edit a node and save revisions. so, the log message should not be displayed in this case.

i really like puregin's idea of handling the log message field as part of validation, since that avoids the complication of trying to figure out if a user could enable revisions or not, and leaves it squarely in the land of reality: if we're saving a new revision we offer a log message field. i don't think we should make this field required, and i don't know if that complicates the validation step, but otherwise, i think this would be a pretty straightforward solution that avoids other possible complications and unexpected behavior.

if anyone other than the two of us thinks this is a good idea ;) and no one gets to it first, i'll try to make a patch for this part.

thanks,
-derek

Zen’s picture

FileSize
1.44 KB

Attached patch adds a comment.

-K

dww’s picture

Status: Needs review » Reviewed & tested by the community

who am i to say, but i think this is RTBC now... patch looks fine, is tested, and even has a few comments. ;)

(please reset to "active/normal" once committed, since there are other (non-critical) parts of this issue that the patch doesn't address).

killes@www.drop.org’s picture

Priority: Critical » Normal
Status: Reviewed & tested by the community » Active

patch by Zen committed.

I don't think this was critical. Setting to active to discuss further issues.

dww’s picture

Assigned: Zen » dww

the "pave the way for CCK" patch (http://drupal.org/node/62340) managed to wipe out the 'Log message' field from page nodes. story already lost this field in the conversion to 4.7 FAPI. i think this is turning into an urgent problem (though it's not really critical in the sense that people's sites don't work, etc).

i'd love to see this fixed in a global way before the next code freeze. the 2-page form seems like it might be the way to go. also, given recent changes to FAPI (http://drupal.org/node/74660) the 2-page form might be a lot easier to do. i'll try to get some direction from chx/eaton before i dig in deeper and roll the patch.

dww’s picture

Version: x.y.z » 5.x-dev
Status: Active » Closed (fixed)

i think all the remaining problems here were fixed by http://drupal.org/node/100850.