I am working on porting uc_donation to Drupal 7 and ran into a glitch:
function uc_product_get_picture($nid, $style = 'uc_product') {
$product = node_load($nid);
$field_name = variable_get('uc_image_' . $product->type, 'uc_product_image');
if (isset($product->{$field_name})) {
return field_view_field('node', $product, $field_name, array(
'label' => 'hidden',
'type' => 'image',
'settings' => array(
'image_link' => 'content',
'image_style' => $style,
),
), $product->language);
}
else {
return array();
}
}
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 1106594-uc_product_get_picture.patch | 1.94 KB | longwave |
| #7 | 1106594_image_langcode.patch | 874 bytes | Island Usurper |
| #5 | 1106594.patch | 757 bytes | tr |
| uc_product_dynamic_image_field.patch | 1017 bytes | noahlively |
Comments
Comment #1
longwaveCommitted, with field_view_field() changed to field_view_value().
Comment #3
tr commentedThis fix isn't quite right. Reopening. See also the related issue fixed by the same patch, #1119396: Error when product is added in cart.
Background: I have a test site where I've created a bunch of products. Most of my products show up as 'Language neutral' (see admin/content) but at some time around when this patch was applied, newly-created products are now showing up as 'English'. This might have been caused by something in Drupal core, as I'm running against Drupal 7.x HEAD. I haven't installed or used anything having to do with translation or localization.
Although I don't know why my default product language has changed from 'Language neutral' to 'English', the impact is to break product image displays in the cart context (/cart) and in the store product list (/admin/store/products/view). (By "break" I mean the images don't show up at all in these contexts for 'English' products.) These two places are the ONLY pages that use uc_product_get_picture() for image display - the /products page and the /catalog both use Views, which doesn't call uc_product_get_picture(). The product page itself, /node/%, also doesn't use uc_product_get_picture().
My analysis is that Ubercart is creating the image field as a language neutral field (indexed by 'und'). Commit aaea4cf, which applied a modified version of the above patch, changed uc_product_get_picture() to return an image indexed by the product language. For 'Language neutral' products, this works. For 'English' products, it doesn't; it doesn't work because in the code
$product->uc_product_image[$product->language][0]$product->language is now 'en' whereas Ubercart has only created a language neutral image, which is accessed by $product->language = 'und' instead.I've attached a patch which fixes the problem for me - it shows product images at /cart and /admin/store/products/view for both my 'Language neutral' products and my 'English' products. As I don't completely understand the whole issue of language for fields, I'm not positive that this will work for all use cases. However, it seems an improvement over what's in there now.
Upgrading this to 'major'.
Comment #4
tr commentedTagging.
Comment #5
tr commented... and I forgot the patch.
Comment #6
longwaveHaven't tested this, but I think we should use the LANGUAGE_NONE constant instead of the 'und' string.
Comment #7
Island Usurper commentedHow about we go all the way and figure out which language it's supposed to be? I think it's possible that some fields can be set to have different translations, so we can't guarantee that the right product image will always be under 'und'. However, field_view_value() calls field_language() to figure out what the right language fallback ought to be, so it seems reasonable that we can, too, to get the right value out of the product.
Works for my new, English language products and undetermined-language images.
Comment #8
longwaveThis function needs to guard against an empty $field_name, which was the cause of #1129516: Fatal Error when clicking on "add to cart"
Comment #9
longwavefield_view_field() handles field language selection, but returns all images when we only want one here. But we can switch back to that and borrow the code from field_view_value() that returns just the first image, and seemingly not have to worry about language issues at all. I also fixed the docblock in this patch, as it had the wrong return type and still referred to ImageCache.
Comment #10
Island Usurper commentedAlright. Looks good to me.