Image edits do not take effect after preview [#181498]

From the revision 1.209.2.46 onwards (I backtracked through the CVS history)

- Create an image, preview, save. OK
- Edit the image, choose a different picture.
- Preview : Thumbs are generated and it looks OK
- Save : Old image re-appears, new one is lost.

Expected behaviour - the new image should replace the old one.

When editing and saving directly, no preview, it works correctly.

It looks like all that extra logic that was added to try and avoid rebuilding too much was a bit timid.
Looking into the code a little, I find that by the time it reaches image_update, the $node->images[] are set with the proper new files, but the $node->files[] are not being replaced.

Revision 1.209.2.46 was OK. I guess I should stick with the DRUPAL-5--1-5 release for now.

Comment	File	Size	Author
#1	image_replace_preview_fix.patch	786 bytes	dman

Comments

Comment #1

dman commented 26 November 2007 at 15:44

Version:

5.x-1.x-dev

» 5.x-2.x-dev

Status	File	Size
new	image_replace_preview_fix.patch	786 bytes

Problem remains in latest releases.

Here's my fix.
It appears that the new_file flag was not remaining persistant through previews, so the temp files were not getting migrated into the permanent location at the end of the process.
Replacement images vanished when you tried to save them (although saving immediately without preview still worked)

Something about the way form API handles #type=value elements.
I changed the form so the element is always there whether it's set or not, and had to make it 'hidden' not 'value'. for some reason 'value' just isn't working as expected.

Anyway, now you can actually replace existing images with other ones again.

-  if ($node->new_file) {
-    $form['new_file'] = array('#type' => 'value', '#value' => TRUE);
-  }
-
+  $form['new_file'] = array('#type' => 'hidden');
+  if($node->new_file) $form['new_file']['#value'] = TRUE;

Comment #2

dman commented 26 November 2007 at 15:46

Status:

Active

» Needs review

it's a patch

Comment #3

drewish commented 27 November 2007 at 17:11

does it make sense to just always assigning a value? :

$form['new_file'] = array(
  '#type' => 'hidden',
  '#value' => isset($node->new_file) ? $node->new_file : FALSE,
);

Comment #4

dman commented 27 November 2007 at 20:43

You'd think so, but no! try it and see.
I went through a dozen variations trying to find what worked and what didn't.

The problem is that here at form build time, $node->new_file has not been set.
However, sometime later, the value DOES get correctly assigned, and it works as expected.
BUT if it didn't exist (like before) or we've manually set it to FALSE - like that example, the value does not get retained ... and changes to the image do not get saved later.

My way only sets it if it needs to be set, and inheirits it the rest of the time, never unsetting it to FALSE.

It's probably a case for #default_value, but no, that didn't work either.

There probably is something in the formapi pipeline that I'm missing, but for now this is broken and this patch does fix it.

If there is a more elegant solution to why this is even a problem ($node is not properly initialized with the $_POST data at the time image_form() is called) I'd be interested to find out.