File changes not remembered on postback

List problem fixed. Please review.

Because it has been a while since the last follow up, I take the liberty of improving Souvent22's patch.

The problem with Souvent22's patch was that the query that selects files by nid returns all files from all revisions. I rewrote this query to only return files from the last revision using an inner join. This seems to fix the phantom 0 byte files problem, as well as the problem of reviving files that were removed in previous revisions.

Another problem that I fixed is that the delete, list and description form fields were not remembered when previewing a node. I don't believe an issue has been posted for this problem yet, so I fitted it into this issue.

Updated patch, fixed a bug which caused the list checkbox not to be enabled by default.

Yes, you're right. The problem was that new files were attempted to be copied from the previous revision to the new revision in upload_save, so I added a check for that.

I've also changed upload_nodeapi($op = 'view') a bit, so that when one or more delete checkboxes are enabled and you click preview, they don't show up in the node preview anymore.

Nice, thank you! I've also tested it, its even used on a production site, and I have not came across any problems (yet).

The new code in upload_load does not make sense to me.

+ // not a revision. You view and/or revert to a revision.
+ if ($node->vid || $node->nid) {

A node either has vid and nid or none of them.

+ if ($node->vid) {
+ $result = db_query("SELECT * FROM {files} WHERE vid = %d", $node->vid);
+ }
+ else {
+ $result = db_query("SELECT f.* FROM {files} f INNER JOIN {node} n ON f.nid = n.nid WHERE f.nid = %d AND f.vid = n.vid", $node->nid);
+ }

Why do you treat those cases differently?

A node either has vid and nid or none of them.

That is not the case when the node is being edited. Then only the nid is set, and I am certain this is true, because if it weren't true, the second query in the else part would never happen, and it does. Exactly as Souvent22 says in his comment:

When you are editing, you are editing the node not a revision. You view and/or revert to a revision.

Forgot to add this... what the second query actually does is load the downloads from the last revision.

If your statement where true we'd have a more serious bug.

If you create a new node, it will have both vid and nid after you save it. regardless of the revisions setting.
If you then later edit it again, then your loaded node object shoudl have both vid and nid. teh difference between "revisions on" and "revisions off" is that in the first case you will get a _new_ vid for the next revision which will be the current version ID for all further node_loads.

You are very right, this is a serious bug in node.module indeed! Only the nid is remembered at postback through a hidden form field, but vid is not. I've submitted an issue (http://drupal.org/node/33209).

But the other fixes to the upload module still stand, like the improved behavior when previewing a node. Updated patch attached.

There have been some significant changes in the upload module's code, mostly because of the new forms api. The patch should therefore be remade. Unfortunately, I am not using the newest CVS yet because of the forms api changes, so I am not really into the new forms api and someone else should adapt my patch. Thank you.

This might be fixed in CVS HEAD thanks to a patch being committed 2 minutes ago. Please verify and update this issue accordingly. Thanks. Looks like a 100% different patch though.