Attached image gets disconnected when nodes are updated somewhere other than a node form

Yes I have a patch for this fix to image_attach, first noticed in #632828: Disconnects attached images when node is published. Will post it tomorrow, not got time tonight.

Jonathan

The problem occurred when Image changed to multiple images. Scheduler calls node_save() to save the node with a new status value, which invokes image_attach_nodeapi() with op='update'. The existing code calls unset($node->iids[0]) to remove the '-none-' selection item from the form. However, during other operations, eg in node_save() the image attach iid array is zero-keyed, so doing unset($node->iids[0]) detaches the first image.

The fix is to test the value of $node->iids[0] and only do the unset if the value is 0, which it will be only when saving the node via the edit form.

The patch was created against a dev version from a couple of weeks ago, so you may get an offset warning when applying it. If you would like me to re-roll the patch let me know and I'll download the latest dev.

Jonathan

Good catch! Thanks for reporting this and doing the detective work and the patch!

Just to check I understand this right: the only time we get $node->iids keyed by image node id is when we're coming from a node save form, and the rest of the time that array is keyed in numerical sequence?
If that is the case, then I think it would be cleaner to deal with this little bit of form value cleanup in a submit handler we add to the form, and that way come into hook_nodeapi with clean values in all cases.

I can reproduce the bug with the following code in the Execute PHP block:

$node = node_load(1); // where 1 is the nid of a node with several attached images
node_save($node);

Then go see the node (you have to load the node after this code has run; you can't run it on the node page itself).
The first image has gone.

Here's a patch implementing the approach outline in #4.

Could either of you test this please, both with the scheduler setup and regular image attach node tasks?

Better title to prevent duplicates; marking as critical.

I'd like to get this committed and a new beta released with the fix. Please test the patch!

I will test this at the weekend (hopefully Saturday) sorry can't do it before then.

Jonathan

Hi Joachim,
I cannot say for definite that everywhere except node-edit the array is 0-keyed, but each time I have investigated, it has been that way. So I think this is a better way to fix it, and image_attach_nodeapi does not have to do any clean-up.

I have tested this patch with scheduler and with ordinary adding images, selecting -none-, selecting -none- and an image, etc. It works fine, so I would say apply it.

Jonathan

Committed to CVS.

Given this is a data loss bug, I'm going to roll another beta release with this in, probably tomorrow.

Thanks qwerdox for reporting, and jonathan1055 for figuring it out and testing -- this makes my job as maintainer a whole lot easier and I really appreciate it :D