Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Hi,
I have just upgraded from alpha5 to beta3. Great work on the enhancements and fixes, thanks.
When editing a node, on preview I noticed that the attached image is not being shown in the teaser, but it is shown when the saved node is displayed in teaser from, eg on front page. I tracked through the code and have found where the problem lies - it seems that in some places the array of image ids, $node->iids, is keyed from 0, eg 0=>77, 1=>76 for two images 77 and 76, but in other places it is keyed with the image id, eg 77=>77, 76=>76. The attached screen grabs illustrate this.
- #1 is from a blog node when displayed on the front page. In image_attach_nodeapi() the array is keyed 0,1 and the first image $node->iids[0] is correctly passed to theme_image_attach_teaser()
- #2 is the same node in full page, with both image ids passed in turn to theme_image_attach_teaser()
- #3 shows the values after entering edit mode.
- #4 is a preview during editing. Now the image array $node->iids is keyed on the image id, which is fine for the full page, because each node is processed with a foreach loop. But when the teaser is rendered with image_attach_nodeapi $node->iids[0] does not exist, so no second parameter is passed to theme_image_attach_teaser() and the image is not rendered.
If you want to replicate this error, enable image_attach for the blog node type, create a new post, attach an image, and preview it. If anyone can reproduce this it would be useful, as I don't think is is anything odd with my site, but it just could be.
I have not yet worked out why the image array is 0-keyed in nodeapi_image_attach when rendered for viewing (picture 1) but keyed on image id when rendered for edit preview (picture). op=view and teaser=1 in both cases. I think it must be something to do with $node->build_mode which is 0 for normal viewing of the teaser on the front page, but 1 when generating the edit preview, as shown in the node_build_content() debug.
However, a simple fix in image_attach_nodeapi() is to use current($node->iids) instead of $node->iids[0] as this will always return an item in this situation.
The attached patch makes the change. Hope this is helpful.
Jonathan
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | _image_attach.teaser_firstiid.patch.txt | 714 bytes | jonathan1055 |
| _image_attach.teaser_image.patch.txt | 695 bytes | jonathan1055 | |
| image_attach blog 4 preview.jpg | 315.05 KB | jonathan1055 | |
| image_attach blog 3 on edit.jpg | 18.04 KB | jonathan1055 | |
| image_attach blog 2 full node.jpg | 59.09 KB | jonathan1055 |
Comments
Comment #1
joachim CreditAttribution: joachim commentedGood catch!
And thanks for doing the detective work :)
I'm able to reproduce this, but you need enough text to cause the trimmed preview to be shorter than the full node.
The reason for the difference in keying is that a select box form item in FormAPI returns an array whose keys are the ids from the things you're selecting, and whose values are 0 for unselected and the same id again if selected.
So:
7 => 0
8 => 8
means the select box has nodes 7 and 8, and only 8 is selected.
But when we are looking at a rendered node, the attached images are keyed by weight, so 0-n.
I'd rather use something other than count(), because although we're unlikely to mess with the array pointer, it doesn't feel like a robust way to get the first value.
A quick read of PHP docs suggests reset(). (Which strikes me as ugly. Why a language with 5 million functions can't spare a synonym likst 'first' I don't know.)
It's a shame we can't AFAIK detect when the node is in preview.
Something like this seems to me to be most readable:
Will reroll this when I get a moment.
Comment #2
jonathan1055 CreditAttribution: jonathan1055 commentedYes, you are right, creating $first_iid is clearer and safer. Here is a patch that does exactly the code change above, to make it easier for others to test, and for you to implement. It is rolled against dev of 2009-11-07
Jonathan
Comment #3
joachim CreditAttribution: joachim commentedThanks for rolling the patch again!
Works perfectly for me.
I've added a comment to about the use of reset to prevent future confusion about its reason for being there, and committed to CVS.
#630546 by jonathan1055: Fixed attached image not appearing in teaser preview.