Issue #2346893 by lauriii, idebr, slashrsm, RavindraSingh, Rade, Fabianx, alexpott, swentel, gauravjeet, darrenwh, deepak_zyxware, joelpittet, Wim Leers, Yogesh Pawar, Vj, ivan.chavarro, josephdpurcell, josmera01, rloos289, kattekrab, Tanvish Jha, csakiistvan, xjm, larowlan, akalata: Duplicate AJAX wrapper around a file field

core/lib/Drupal/Core/Render/Renderer.php

@@ -509,11 +509,17 @@ class Renderer implements RendererInterface {
     // We store the resulting output in $elements['#markup'], to be consistent
     // with how render cached output gets stored. This ensures that placeholder
     // replacement logic gets the same data to work with, no matter if #cache is
-    // disabled, #cache is enabled, there is a cache hit or miss.
+    // disabled, #cache is enabled, there is a cache hit or miss. If
-    $prefix = isset($elements['#prefix']) ? $this->xssFilterAdminIfUnsafe($elements['#prefix']) : '';
+    // #render_children is set the #prefix and #suffix will have already been
-    $suffix = isset($elements['#suffix']) ? $this->xssFilterAdminIfUnsafe($elements['#suffix']) : '';
+    // added.
-
+    if (isset($elements['#render_children'])) {
-    $elements['#markup'] = Markup::create($prefix . $elements['#children'] . $suffix);
+      $elements['#markup'] = Markup::create($elements['#children']);
+    }
+    else {
+      $prefix = isset($elements['#prefix']) ? $this->xssFilterAdminIfUnsafe($elements['#prefix']) : '';
+      $suffix = isset($elements['#suffix']) ? $this->xssFilterAdminIfUnsafe($elements['#suffix']) : '';
+      $elements['#markup'] = Markup::create($prefix . $elements['#children'] . $suffix);
+    }
 
     // We've rendered this element (and its subtree!), now update the context.
     $context->update($elements);

core/modules/file/src/Tests/FileFieldValidateTest.php

@@ -187,4 +187,25 @@ class FileFieldValidateTest extends FileFieldTestBase {
     $this->assertText('Article ' . $node->getTitle() . ' has been updated.');
   }
 
+  /**
+   * Test the validation message is displayed only once for ajax uploads.
+   */
+  public function testAJAXValidationMessage() {
+    $field_name = strtolower($this->randomMachineName());
+    $this->createFileField($field_name, 'node', 'article');
+
+    $this->drupalGet('node/add/article');
+    /** @var \Drupal\file\FileInterface $image_file */
+    $image_file = $this->getTestFile('image');
+    $edit = [
+      'files[' . $field_name . '_0]' => $this->container->get('file_system')->realpath($image_file->getFileUri()),
+      'title[0][value]' => $this->randomMachineName(),
+    ];
+    $this->drupalPostAjaxForm(NULL, $edit, $field_name . '_0_upload_button');
+    $elements = $this->xpath('//div[contains(@class, :class)]', [
+      ':class' => 'messages--error',
+    ]);
+    $this->assertEqual(count($elements), 1, 'Ajax validation messages are displayed once.');
+  }
+
 }

core/modules/image/src/Tests/ImageFieldValidateTest.php

@@ -161,4 +161,26 @@ class ImageFieldValidateTest extends ImageFieldTestBase {
     ];
   }
 
+  /**
+   * Test the validation message is displayed only once for ajax uploads.
+   */
+  public function testAJAXValidationMessage() {
+    $field_name = strtolower($this->randomMachineName());
+    $this->createImageField($field_name, 'article', ['cardinality' => -1]);
+
+    $this->drupalGet('node/add/article');
+    /** @var \Drupal\file\FileInterface[] $text_files */
+    $text_files = $this->drupalGetTestFiles('text');
+    $text_file = reset($text_files);
+    $edit = [
+      'files[' . $field_name . '_0][]' => $this->container->get('file_system')->realpath($text_file->uri),
+      'title[0][value]' => $this->randomMachineName(),
+    ];
+    $this->drupalPostAjaxForm(NULL, $edit, $field_name . '_0_upload_button');
+    $elements = $this->xpath('//div[contains(@class, :class)]', [
+      ':class' => 'messages--error',
+    ]);
+    $this->assertEqual(count($elements), 1, 'Ajax validation messages are displayed once.');
+  }
+
 }

core/tests/Drupal/KernelTests/Core/Render/RenderTest.php

@@ -16,7 +16,7 @@ class RenderTest extends KernelTestBase {
    *
    * @var array
    */
-  public static $modules = ['system', 'common_test'];
+  public static $modules = ['system', 'common_test', 'theme_test'];
 
   /**
    * Tests theme preprocess functions being able to attach assets.
@@ -43,6 +43,23 @@ class RenderTest extends KernelTestBase {
     \Drupal::state()->set('theme_preprocess_attached_test', FALSE);
   }
 
+  /**
+   * Ensures that render array children are processed correctly.
+   */
+  public function testRenderChildren() {
+    // Ensure that #prefix and #suffix is only being printed once since that is
+    // the behaviour the caller code expects.
+    $build = [
+      '#type' => 'container',
+      '#theme' => 'theme_test_render_element_children',
+      '#prefix' => 'kangaroo',
+      '#suffix' => 'kitten',
+    ];
+    $this->render($build);
+    $this->removeWhiteSpace();
+    $this->assertNoRaw('<div>kangarookitten</div>');
+  }
+
   /**
    * Tests that we get an exception when we try to attach an illegal type.
    */

core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php

@@ -291,6 +291,42 @@ class RendererBubblingTest extends RendererTestBase {
     ];
     $data[] = [$test_element, ['bar', 'foo'], $expected_cache_items];
 
+    // Ensure that bubbleable metadata has been collected from children and set
+    // correctly to the main level of the render array. That ensures that correct
+    // bubbleable metadata exists if render array gets rendered multiple times.
+    $test_element = [
+      '#cache' => [
+        'keys' => ['parent'],
+        'tags' => ['yar', 'har']
+      ],
+      '#markup' => 'parent',
+      'child' => [
+        '#render_children' => TRUE,
+        'subchild' => [
+          '#cache' => [
+            'contexts' => ['foo'],
+            'tags' => ['fiddle', 'dee'],
+          ],
+          '#attached' => [
+            'library' => ['foo/bar']
+          ],
+          '#markup' => '',
+        ]
+      ],
+    ];
+    $expected_cache_items = [
+      'parent:foo' => [
+        '#attached' => ['library' => ['foo/bar']],
+        '#cache' => [
+          'contexts' => ['foo'],
+          'tags' => ['dee', 'fiddle', 'har', 'yar'],
+          'max-age' => Cache::PERMANENT,
+        ],
+        '#markup' => 'parent',
+      ],
+    ];
+    $data[] = [$test_element, ['foo'], $expected_cache_items];
+
     return $data;
   }
 

core/tests/Drupal/Tests/Core/Render/RendererTest.php

@@ -403,6 +403,25 @@ class RendererTest extends RendererTestBase {
     };
     $data[] = [$build, 'baz', $setup_code];
 
+    // #theme is implemented but #render_children is TRUE. In this case the
+    // calling code is expecting only the children to be rendered. #prefix and
+    // #suffix should not be inherited for the children.
+    $build = [
+      '#theme' => 'common_test_foo',
+      '#children' => '',
+      '#prefix' => 'kangaroo',
+      '#suffix' => 'unicorn',
+      '#render_children' => TRUE,
+      'child' => [
+        '#markup' => 'kitten',
+      ],
+    ];
+    $setup_code = function () {
+      $this->themeManager->expects($this->never())
+        ->method('render');
+    };
+    $data[] = [$build, 'kitten', $setup_code];
+
     return $data;
   }
 
@@ -562,25 +581,81 @@ class RendererTest extends RendererTestBase {
   }
 
   /**
-   * Tests that a first render returns the rendered output and a second doesn't.
+   * Tests rendering same render array twice.
    *
-   * (Because of the #printed property.)
+   * Tests that a first render returns the rendered output and a second doesn't
+   * because of the #printed property. Also tests that correct metadata has been
+   * set for re-rendering.
    *
    * @covers ::render
    * @covers ::doRender
+   *
+   * @dataProvider providerRenderTwice
    */
-  public function testRenderTwice() {
+  public function testRenderTwice($build) {
-    $build = [
+    $this->assertEquals('kittens', $this->renderer->renderRoot($build));
-      '#markup' => 'test',
+    $this->assertEquals('kittens', $build['#markup']);
-    ];
+    $this->assertEquals(['kittens-147'], $build['#cache']['tags']);
-
-    $this->assertEquals('test', $this->renderer->renderRoot($build));
     $this->assertTrue($build['#printed']);
 
     // We don't want to reprint already printed render arrays.
     $this->assertEquals('', $this->renderer->renderRoot($build));
   }
 
+  /**
+   * Provides a list of render array iterations.
+   *
+   * @return array
+   */
+  public function providerRenderTwice() {
+    return [
+      [
+        [
+          '#markup' => 'kittens',
+          '#cache' => [
+            'tags' => ['kittens-147']
+          ],
+        ],
+      ],
+      [
+        [
+          'child' => [
+            '#markup' => 'kittens',
+            '#cache' => [
+              'tags' => ['kittens-147'],
+            ],
+          ],
+        ],
+      ],
+      [
+        [
+          '#render_children' => TRUE,
+          'child' => [
+            '#markup' => 'kittens',
+            '#cache' => [
+              'tags' => ['kittens-147'],
+            ],
+          ],
+        ],
+      ],
+    ];
+  }
+
+  /**
+   * Ensures that #access is taken in account when rendering #render_children.
+   */
+  public function testRenderChildrenAccess() {
+    $build = [
+      '#access' => FALSE,
+      '#render_children' => TRUE,
+      'child' => [
+        '#markup' => 'kittens',
+      ],
+    ];
+
+    $this->assertEquals('', $this->renderer->renderRoot($build));
+  }
+
   /**
    * Provides a list of both booleans.
    *