image_attach support for i18n [#1745878]

In its current state image_attach does not support i18n (i18ncontent).
When adding a new translation for a node, the images attached in the original node are not part of the new form used to add a translation (but other fields are preserved)

The problem comes from image_attach_form_alter where $has_existing_images = FALSE because $node->iids is empty.
(while $node->translation_source->iids contains the requested values).

It should be noted that when using i18n, image_attach_nodeapi is given an additional round-trip where $op = 'prepare translation'.
That's where a module is expected to switch the $node->translation_source attributes back into $node so that it can play with the values later on in the form processing.

At least, that's what cck/content.module does in order to play well with i18n.

Comment	File	Size	Author
#4	1745878-i18n-support-4.patch	1.12 KB	drzraf
#4
#1	1745878-image_attach-i18n.patch	636 bytes	drzraf
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

drzraf CreditAttribution: drzraf commented 23 August 2012 at 08:28

Status:

Active

» Needs review

File	Size
1745878-image_attach-i18n.patch	636 bytes

patch, based on the cck way of integrating with i18n.

Comment #2

drzraf CreditAttribution: drzraf commented 10 December 2012 at 21:26

ping

Comment #3

joachim CreditAttribution: joachim commented 13 December 2012 at 08:48

Status:

Needs review

» Needs work

Thanks for working on this.

A few things this patch needs, though:

- Dreditor review indicates a whitespace problem which is probably tabs instead of spaces
- I'd like a comment after the 'case' to explain what is being done. I don't know enough about the translation API to follow what the code is doing.

Lastly, review from another user of translation with image nodes would be nice (if unlikely...).

Comment #4

drzraf CreditAttribution: drzraf commented 13 December 2012 at 10:43

Status:

Needs work

» Needs review

File	Size
1745878-i18n-support-4.patch	1.12 KB

Comment #5

13 December 2012 at 10:45

Status:

Needs review

» Needs work

The last submitted patch, 1745878-i18n-support-4.patch, failed testing.

Comment #6

drzraf CreditAttribution: drzraf commented 13 December 2012 at 10:56

Comment #7

joachim CreditAttribution: joachim commented 13 December 2012 at 11:37

Indeed -- that looks like the testbot being wacky.

However, a few things need tidying up with the patch:

+++ b/contrib/image_attach/image_attach.module
@@ -473,6 +473,17 @@ function image_attach_nodeapi(&$node, $op, $teaser, $page) {
+    /*
+      When using i18ncontent, the 'prepare translation' hook_nodeapi round-trip
+      expects the copy of $node->translation_source attributes back into $node
+      For more information: #1745878, example: cck/content.module
+    */

These should use // not /** */.

Also, give the full URL if you feel its needed -- though I think we can probably do without it.

+++ b/contrib/image_attach/image_attach.module
@@ -473,6 +473,17 @@ function image_attach_nodeapi(&$node, $op, $teaser, $page) {
+	$node->iids = $node->translation_source->iids;

There's still a tab here.