It appears the nodeapi 'view' hook receives the 'files' as arrays instead of objects during node preview. So theme_upload_attachments gets FALSE for $file->list (even when $file['list'] is true) and doesn't display. After casting to object, the $href definition checks $file->fid.. for a newly uploaded file this is something like 'upload_0' so I changed this to a strpos check. Finally, l($text, $href) is called when $href is already an url (not a path) and $text is already check_plain'ed. Patch attached, hope this helps.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | upload_preview_patch_0.txt | 1.84 KB | mindless |
| #10 | upload_preview_patch.txt | 979 bytes | mindless |
| upload.module_patch.txt | 901 bytes | mindless |
Comments
Comment #1
mindless commentedmaybe also change if ($file->list) to if ($file->list && empty($file->remove)) so the preview doesn't show attachments you selected for removal.
Comment #2
mindless commentedBump.. I hope someone can review this patch, thanks.
Comment #3
gerhard killesreiter commentedmoving this to CVS. It should be reviewed there, applied and backported, if neccessary.
Comment #4
beginner commentedbookmarking. I'll try to review later this week.
Comment #5
beginner commentedOk, the patch looks good overall, but I have a problem with the url created for an uploaded file.
I am getting urls like:
http://127.0.0.1/drupal/70742-cvs/?q=/drupal/70742-cvs/?q=files/robots.txt.so the url formatting needs work.
Also, can you explain what exactly happens to the files? They are not already uploaded, are they? They are still in the /tmp directory. So what would happen if during the preview I click on the url for the file that I am uploading?
Wouldn't it be more accurate NOT to print a link, but to print the file name with the mention that it is uploading?
Comment #1 is valid, and fix may be included in this patch.
Comment #6
mindless commentedI only changed the conditional to decide which form to use.. I don't know why you see a URL like that.. is fixing this part of the code I didn't touch required to get this fix put into CVS?
The uploaded file is in a tmp location as you suggest.. I believe the links do work (see "Add handlers for previewing new uploads" comment in upload_menu), except in this case: http://drupal.org/node/68685
Let me know how I can help get this fix accepted, thanks.
Comment #7
beginner commentedDoes the patch in the following issue fix the current issue?
http://drupal.org/node/104047 node previews broken by missing hook_submit
Comment #8
mindless commentedI rolled back my patch and applied that one.. new file attachments NOT shown in node preview. Added back my patch, worked again.
Comment #9
beginner commentedGood to know. Thanks.
Comment #10
mindless commentedRevisited this and fixed the problem mentioned in comment #5 (found other code in upload.module that does correctly what theme_upload_attachments had wrong in creating the url). Updated patch against current HEAD rev of upload.module is attached. Please review, thanks!
Comment #11
ChrisKennedy commentedPatch works for me.
However, there should be a comment to explain the use of the strpos() check, and the patch should be re-rolled from the Drupal root directory.
Comment #12
mindless commentedNew patch attached, thanks for testing.
Comment #13
dries commentedCommitted to CVS HEAD.
Neil: this probably needs to be verified and committed against DRUPAL-5.
Comment #14
drummCommitted to 5.
This might need to go in 4.7
Comment #15
killes@www.drop.org commentedbackported.
Comment #16
(not verified) commented