Notices are thrown while creating a node translation if the source field is empty. Moreover if a plaintext textfield is attached to the node, the corresponding widget is not populated.

    *  Notice: Undefined index: en in text_field_prepare_translation() (line 643 of E:\www\test\modules\field\modules\text\text.module).
    * Warning: Invalid argument supplied for foreach() in text_field_prepare_translation() (line 643 of E:\www\test\modules\field\modules\text\text.module).
    * Notice: Undefined index: in text_field_prepare_translation() (line 645 of E:\www\test\modules\field\modules\text\text.module).
    * Notice: Trying to get property of non-object in filter_access() (line 931 of E:\www\test\modules\filter\filter.module).
CommentFileSizeAuthor
#5 textfield_transalation-879910-5.patch3.96 KBplach
#4 textfield_transalation-879910-4.patch4.57 KBplach
#2 textfield_transalation-879910-2.patch4.56 KBplach
#1 textfield_transalation-879910-1.patch4.62 KBplach

Comments

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Active » Needs review
Issue tags: +Quick fix
StatusFileSize
new textfield_transalation-879910-1.patch4.62 KB

We were missing a couple of if. Tests are included.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
StatusFileSize
new textfield_transalation-879910-2.patch4.56 KB 
+++ modules/field/modules/text/text.test	11 Aug 2010 11:03:17 -0000
@@ -401,11 +401,66 @@ class TextTranslationTestCase extends Dr
+  function testTextField() {

The PHPDoc was missing.

Powered by Dreditor.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Needs work 
+++ modules/field/modules/text/text.module	11 Aug 2010 11:17:09 -0000
@@ -639,11 +639,13 @@ function text_field_prepare_translation(
+    foreach ($source_entity->{$field_name}[$source_langcode] as $delta => $item) {
+      $format_id = $item['format'];
+      if (!empty($format_id) && !filter_access($formats[$format_id])) {

+++ modules/field/modules/text/text.test	11 Aug 2010 11:18:38 -0000
@@ -402,10 +402,68 @@ class TextTranslationTestCase extends Dr
+    $instance = array(
...
+      'settings' => array(
+        'text_processing' => FALSE,

Normally, a field without text processing shouldn't have a 'format' field value item key at all, no?

I would have expected an exception on this line:

$format_id = $item['format'];

+++ modules/field/modules/text/text.test	11 Aug 2010 11:18:38 -0000
@@ -402,10 +402,68 @@ class TextTranslationTestCase extends Dr
+    // Setup a field and instance

Why do we have to create a new field and instance? Isn't the regular body field sufficient?

+++ modules/field/modules/text/text.test	11 Aug 2010 11:18:38 -0000
@@ -402,10 +402,68 @@ class TextTranslationTestCase extends Dr
+    // Login as translator.
+    $this->drupalLogout();
+    $this->drupalLogin($this->translator);

Can we move this above "// Create content", so as to keep $edit and drupalPost() together?

Also, drupalLogout() does not need to be called manually.

Powered by Dreditor.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Needs review
StatusFileSize
new textfield_transalation-879910-4.patch4.57 KB

Implemented #3.

Normally, a field without text processing shouldn't have a 'format' field value item key at all, no?
I would have expected an exception on this line:
$format_id = $item['format'];

Well, I don't know if we want to open a separate issue, but in this case $item['format'] is NULL.

Why do we have to create a new field and instance? Isn't the regular body field sufficient?

Body is a long text, it can't have plain text processing.

Edit: Totally FALSE, my bad, I checked the wrong Field UI page :)

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
StatusFileSize
new textfield_transalation-879910-5.patch3.96 KB

Here it is the patch with the body instead of the custom field in the test.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Thanks!

Not to hold off this fix, but in the future, it would be a good to group logically related code together, i.e.:

+    // Login as translator and create an article.
+    $this->drupalLogin($this->translator);
+    $langcode = LANGUAGE_NONE;
+    $body = $this->randomName();
+    $edit = array(
+      "title" => $this->randomName(),
+      "language" => 'en',
+      "body[$langcode][0][value]" => $body,
+    );
+    $this->drupalPost('node/add/article', $edit, t('Save'));
+    $node = $this->drupalGetNodeByTitle($edit['title']);
+
+    // Translate the article into French.
+    $this->drupalGet("node/$node->nid/translate");
...

So each comment forms a chapter, unit, or "single task" within the test procedure.

Powered by Dreditor.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

yched’s picture

yched commented

The patch in #447816-153: WCAG violation: Relying on a color by itself to indicate a field validation error got committed along with this one : http://drupal.org/cvs?commit=408732

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Quick fix

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.