If a node is previewed while editing and attaching new files, any new attachments are lost when finally saving.

How to reproduce (Drupal 6.5)

1. Go to the edit form of a node.
2. Select a file for attachment.
3. Click the Preview-button.
The form reloads. The newly attached file is visible in the list of attachmens. If you investigate the database it has been saved in the {files} table with with status=0, and has not been saved in the {upload} table yet. On disk the file has been saved.
4. Click the Save-button.
The form reloads. The file no longer appears in the list of attachments. The {files}.status has been changed to 1, but no entry has been created in {upload}.
I would have expected an entry to be created in {upload} table when I clicked Save, so that the new upload would appear in the list of attachments.

Solution

When a previewed node is submitted, (at least in some case?) the files are represented by arrays rather than objects. A cast from array to object fixes the problem as far as I can tell, see the attached diff. I made a diff against Drupal 7, as it says HEAD should be used when making patches.

The line of code that seems to cause the problem was introduced between these versions. I am using Drupal 6.5, so I only know the bug exists there. (Actually I am using some customizations, but they should not matter.) But as the line causing the problem remains in Drupal 7 I guess it exists there too.

CommentFileSizeAuthor
#8 upload-preview.patch2.58 KBmfb
#6 upload_notice.patch452 byteschx
#5 upload_notice.patch462 byteschx
upload_previewed_D6.diff696 bytesemok
upload_previewed_D7.diff696 bytesemok

Comments

emok’s picture

emok commented

I just realized that PHP version may matter, since this was a type (casting) issue. The server I use runs PHP 4.3.11. Perhaps newer versions allow you to use $a->field even on arrays where $a['field'] should be used.

boombatower’s picture

boombatower commented
Version: 7.x-dev » 6.5
Priority: Critical » Normal
Status: Needs work » Needs review

As of http://testing.drupal.org/pifr/node/1/323438 patches fail to apply... D6 don't worry about as it only tests D7 trying to get d.o updated to that end.

Please make patches from drupal root.

damien tournoud’s picture

damien tournoud commented
Version: 6.5 » 7.x-dev
Priority: Normal » Critical
Status: Needs review » Needs work

Ok, this breaks a key functionality, so raising to critical.

I'm not convince this whole loop:

  if (isset($form_state['values']['files'])) {
    foreach ($form_state['values']['files'] as $fid => $file) {
      $form_state['values']['files'][$fid]['new'] = !empty($form['#node']->files[$fid]->new);
    }
  }

... is even needed, because we already set 'new' and $form_state['value'] above.

This will have to be fixed in D7 first, then trivially backported.

Anonymous’s picture

Anonymous (not verified) commented
Version: 6.5 » 7.x-dev
Priority: Normal » Critical
Status: Needs review » Needs work

See emok's note about PHP version in #1. For 7.x-dev we need PHP 5 for HEAD.

chx’s picture

chx commented
Title: New upload lost if previewing before saving » Upload node link throws notice on preview
Priority: Critical » Normal
Status: Needs work » Needs review
StatusFileSize
new upload_notice.patch462 bytes

The original issue I can't reproduce any more. I found another and patched it.

chx’s picture

chx commented
StatusFileSize
new upload_notice.patch452 bytes

Better patch.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 7.x-dev » 6.x-dev

Upload module was just removed from D7 in favour of filefield. Moving down to D6.

mfb’s picture

mfb
they or he
commented
Title: Upload node link throws notice on preview » New upload lost if previewing before saving
Priority: Normal » Critical
StatusFileSize
new upload-preview.patch2.58 KB

I am reproducing this bug on PHP 5.2.12

I didn't yet track down why the file object is converted to an array after repeated previewing.

mfb’s picture

mfb
they or he
commented

This is probably why the bug appears with PHP 5.2.12: http://bugs.php.net/bug.php?id=50255 In previous versions of PHP, if you try to check the existence of a property on an array with empty() or isset(), you get results as if the array were an object. This is no longer the case as of 5.2.12

GoofyX’s picture

GoofyX commented

The patch in #8 works just fine for me (Drupal 6.16, PHP 5.2.13).

advseb’s picture

advseb commented

subscribe

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

So we have confirmation of the patch in #8 is working and the patch looks good structurally so marking RTBC.

advseb’s picture

advseb commented

Yes, we are using this patch on a rather large site and it is working.

Matthew OMalley’s picture

Matthew OMalley commented

So this seem like a very important issue, causing a number of Drupal sites to not work with basic upload functionality (including some of mine) - yet it's been a month (after RTBC) without a release of a new version of Drupal including the patch. Anyone know when the fix will get released in D6.17?

GoofyX’s picture

GoofyX commented

I pinged Gábor about this, so he will hopefully commit it soon.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Ok, I've added an extra space inbetween the cast and the variable because I believe that is in line with our coding standards. Committed, thanks!

Status: Fixed » Closed (fixed)

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

amitkumarmits@gmail.com’s picture

amitkumarmits@gmail.com commented

I am using the drupal 6.15 . I was not able to show the attachment on the page . I just add these two lines and got the benefits.

upload.module

if (isset($form_state['values']['files'])) {
foreach ($form_state['values']['files'] as $fid => $file) {
$form_state['values']['files'][$fid]['new'] = !empty($form['#node']->files[$fid]->new);
// If the node was previewed prior to saving, $form['#node']->files[$fid]
// is an array instead of an object. Convert file to object for compatibility.
$nodefile = (object) $form['#node']->files[$fid];
$form_state['values']['files'][$fid]['new'] = !empty($nodefile->new);

Thanks a lot.......