diff --git a/core/misc/ajax.es6.js b/core/misc/ajax.es6.js
index fefe9f3031..4cb008ff03 100644
--- a/core/misc/ajax.es6.js
+++ b/core/misc/ajax.es6.js
@@ -1027,22 +1027,53 @@
// $(response.data) as new HTML rather than a CSS selector. Also, if
// response.data contains top-level text nodes, they get lost with either
// $(response.data) or $('
').replaceWith(response.data).
- var $new_content_wrapped = $('').html(response.data);
- var $new_content = $new_content_wrapped.contents();
-
- // For legacy reasons, the effects processing code assumes that
- // $new_content consists of a single top-level element. Also, it has not
- // been sufficiently tested whether attachBehaviors() can be successfully
- // called with a context object that includes top-level text nodes.
- // However, to give developers full control of the HTML appearing in the
- // page, and to enable Ajax content to be inserted in places where
- // elements are not allowed (e.g., within
, , and
- // parents), we check if the new content satisfies the requirement
- // of a single top-level element, and only use the container created
- // above when it doesn't. For more information, please see
- // https://www.drupal.org/node/736066.
- if ($new_content.length !== 1 || $new_content.get(0).nodeType !== 1) {
- $new_content = $new_content_wrapped;
+ // attachBehaviors() can, for the same reason, not be called with a
+ // context object that includes top-level text nodes. Therefore text nodes
+ // will be wrapped with a
element. This also gives themers the
+ // possibility to style the response.
+ var $newContent;
+ // We use the trimmed data to be able to detect cases when the response
+ // has only top-level elements or comment nodes with extra whitespace
+ // around. In that case whitespaces are removed and the response
+ // should not be wrapped.
+ // For exemple:
+ // ' content
' is equivalent to 'content
' and
+ // no extra wrapper will be added.
+ var trimedData = response.data.trim();
+ // List of DOM nodes contained in the response for processing.
+ var responseDataNodes;
+
+ // When 'data' is empty or is only whitespace, manually create the node
+ // array because parseHTML would return null.
+ if (trimedData === '') {
+ responseDataNodes = [document.createTextNode(response.data)];
+ }
+ else {
+ responseDataNodes = $.parseHTML(trimedData, true);
+ }
+ // Check every node for it's type to decide if wrapping is necessary.
+ var onlyElementNodes = responseDataNodes.every(function (element) {
+ return element.nodeType === Node.ELEMENT_NODE || element.nodeType === Node.COMMENT_NODE;
+ });
+
+ // When there are only element and/or comment nodes in the reponse, no
+ // extra wrapping necessary.
+ if (onlyElementNodes) {
+ $newContent = $(trimedData);
+ }
+ // When there are other types of top-level nodes, the response need to be
+ // wrapped.
+ else {
+ // If response.data contains only one TEXT_ELEMENT if the target element
+ // is not styled as a block, response.data will be wrapped with SPAN.
+ if (responseDataNodes.length === 1 || ($wrapper.css('display') !== 'block')) {
+ $newContent = $('');
+ }
+ else {
+ $newContent = $('');
+ }
+ // Use reponse.data to keep whitespace as-is.
+ $newContent.html(response.data);
}
// If removing content from the wrapper, detach behaviors first.
@@ -1057,31 +1088,36 @@
}
// Add the new content to the page.
- $wrapper[method]($new_content);
+ $wrapper[method]($newContent);
// Immediately hide the new content if we're using any effects.
if (effect.showEffect !== 'show') {
- $new_content.hide();
+ $newContent.hide();
}
// Determine which effect to use and what content will receive the
// effect, then show the new content.
- if ($new_content.find('.ajax-new-content').length > 0) {
- $new_content.find('.ajax-new-content').hide();
- $new_content.show();
- $new_content.find('.ajax-new-content')[effect.showEffect](effect.showSpeed);
+ if ($newContent.find('.ajax-new-content').length > 0) {
+ $newContent.find('.ajax-new-content').hide();
+ $newContent.show();
+ $newContent.find('.ajax-new-content')[effect.showEffect](effect.showSpeed);
}
else if (effect.showEffect !== 'show') {
- $new_content[effect.showEffect](effect.showSpeed);
+ $newContent[effect.showEffect](effect.showSpeed);
}
// Attach all JavaScript behaviors to the new content, if it was
// successfully added to the page, this if statement allows
// `#ajax['wrapper']` to be optional.
- if ($new_content.parents('html').length > 0) {
+ if ($newContent.parents('html').length > 0) {
// Apply any settings from the returned JSON if available.
settings = response.settings || ajax.settings || drupalSettings;
- Drupal.attachBehaviors($new_content.get(0), settings);
+ // Attach behaviors to all element nodes.
+ $newContent.each(function () {
+ if (this.nodeType === Node.ELEMENT_NODE) {
+ Drupal.attachBehaviors(this, settings);
+ }
+ });
}
},
diff --git a/core/misc/ajax.js b/core/misc/ajax.js
index 89b47988bc..2ef469724d 100644
--- a/core/misc/ajax.js
+++ b/core/misc/ajax.js
@@ -479,13 +479,34 @@
var effect = ajax.getEffect(response);
var settings;
- var $new_content_wrapped = $('').html(response.data);
- var $new_content = $new_content_wrapped.contents();
+ var $newContent;
- if ($new_content.length !== 1 || $new_content.get(0).nodeType !== 1) {
- $new_content = $new_content_wrapped;
+ var trimedData = response.data.trim();
+
+ var responseDataNodes;
+
+ if (trimedData === '') {
+ responseDataNodes = [document.createTextNode(response.data)];
+ } else {
+ responseDataNodes = $.parseHTML(trimedData, true);
}
+ var onlyElementNodes = responseDataNodes.every(function (element) {
+ return element.nodeType === Node.ELEMENT_NODE || element.nodeType === Node.COMMENT_NODE;
+ });
+
+ if (onlyElementNodes) {
+ $newContent = $(trimedData);
+ } else {
+ if (responseDataNodes.length === 1 || $wrapper.css('display') !== 'block') {
+ $newContent = $('');
+ } else {
+ $newContent = $('');
+ }
+
+ $newContent.html(response.data);
+ }
+
switch (method) {
case 'html':
case 'replaceWith':
@@ -496,23 +517,28 @@
Drupal.detachBehaviors($wrapper.get(0), settings);
}
- $wrapper[method]($new_content);
+ $wrapper[method]($newContent);
if (effect.showEffect !== 'show') {
- $new_content.hide();
+ $newContent.hide();
}
- if ($new_content.find('.ajax-new-content').length > 0) {
- $new_content.find('.ajax-new-content').hide();
- $new_content.show();
- $new_content.find('.ajax-new-content')[effect.showEffect](effect.showSpeed);
+ if ($newContent.find('.ajax-new-content').length > 0) {
+ $newContent.find('.ajax-new-content').hide();
+ $newContent.show();
+ $newContent.find('.ajax-new-content')[effect.showEffect](effect.showSpeed);
} else if (effect.showEffect !== 'show') {
- $new_content[effect.showEffect](effect.showSpeed);
+ $newContent[effect.showEffect](effect.showSpeed);
}
- if ($new_content.parents('html').length > 0) {
+ if ($newContent.parents('html').length > 0) {
settings = response.settings || ajax.settings || drupalSettings;
- Drupal.attachBehaviors($new_content.get(0), settings);
+
+ $newContent.each(function () {
+ if (this.nodeType === Node.ELEMENT_NODE) {
+ Drupal.attachBehaviors(this, settings);
+ }
+ });
}
},
diff --git a/core/modules/system/tests/modules/ajax_test/ajax_test.libraries.yml b/core/modules/system/tests/modules/ajax_test/ajax_test.libraries.yml
index f1c73064bd..772a05f734 100644
--- a/core/modules/system/tests/modules/ajax_test/ajax_test.libraries.yml
+++ b/core/modules/system/tests/modules/ajax_test/ajax_test.libraries.yml
@@ -1,3 +1,8 @@
+ajax_insert:
+ js:
+ js/insert-ajax.js: {}
+ dependencies:
+ - core/drupal.ajax
order:
drupalSettings:
ajax: test
diff --git a/core/modules/system/tests/modules/ajax_test/ajax_test.routing.yml b/core/modules/system/tests/modules/ajax_test/ajax_test.routing.yml
index e8d06c0a9f..89191a1c63 100644
--- a/core/modules/system/tests/modules/ajax_test/ajax_test.routing.yml
+++ b/core/modules/system/tests/modules/ajax_test/ajax_test.routing.yml
@@ -6,6 +6,14 @@ ajax_test.dialog_contents:
requirements:
_access: 'TRUE'
+ajax_test.ajax_render_types:
+ path: '/ajax-test/dialog-contents-types/{type}'
+ defaults:
+ _title: 'AJAX Dialog contents routing'
+ _controller: '\Drupal\ajax_test\Controller\AjaxTestController::renderTypes'
+ requirements:
+ _access: 'TRUE'
+
ajax_test.dialog_form:
path: '/ajax-test/dialog-form'
defaults:
@@ -21,6 +29,13 @@ ajax_test.dialog:
requirements:
_access: 'TRUE'
+ajax_test.insert_links:
+ path: '/ajax-test/insert'
+ defaults:
+ _controller: '\Drupal\ajax_test\Controller\AjaxTestController::insertLinks'
+ requirements:
+ _access: 'TRUE'
+
ajax_test.dialog_close:
path: '/ajax-test/dialog-close'
defaults:
diff --git a/core/modules/system/tests/modules/ajax_test/js/insert-ajax.es6.js b/core/modules/system/tests/modules/ajax_test/js/insert-ajax.es6.js
new file mode 100644
index 0000000000..4c5bb61749
--- /dev/null
+++ b/core/modules/system/tests/modules/ajax_test/js/insert-ajax.es6.js
@@ -0,0 +1,41 @@
+/**
+ * @file
+ * Drupal behavior to attach click event handlers to ajax-insert and
+ * ajax-insert-inline links for testing ajax requests.
+ */
+
+(function ($, window, Drupal, drupalSettings) {
+ 'use strict';
+
+ Drupal.behaviors.insertTest = {
+ attach: function (context, settings) {
+ $('.ajax-insert').once('ajax-insert').on('click', function (event) {
+ event.preventDefault();
+ var ajaxSettings = {
+ url: event.currentTarget.getAttribute('href'),
+ wrapper: 'ajax-target',
+ base: false,
+ element: false,
+ method: event.currentTarget.getAttribute('data-method')
+ };
+ var myAjaxObject = Drupal.ajax(ajaxSettings);
+ myAjaxObject.execute();
+ });
+
+ $('.ajax-insert-inline').once('ajax-insert').on('click', function (event) {
+ event.preventDefault();
+ var ajaxSettings = {
+ url: event.currentTarget.getAttribute('href'),
+ wrapper: 'ajax-target-inline',
+ base: false,
+ element: false,
+ method: event.currentTarget.getAttribute('data-method')
+ };
+ var myAjaxObject = Drupal.ajax(ajaxSettings);
+ myAjaxObject.execute();
+ });
+
+ $(context).addClass('processed');
+ }
+ };
+})(jQuery, window, Drupal, drupalSettings);
diff --git a/core/modules/system/tests/modules/ajax_test/js/insert-ajax.js b/core/modules/system/tests/modules/ajax_test/js/insert-ajax.js
new file mode 100644
index 0000000000..9bc9b8be0a
--- /dev/null
+++ b/core/modules/system/tests/modules/ajax_test/js/insert-ajax.js
@@ -0,0 +1,43 @@
+/**
+* DO NOT EDIT THIS FILE.
+* All changes should be applied to ./modules/system/tests/modules/ajax_test/js/insert-ajax.es6.js
+* See the following change record for more information,
+* https://www.drupal.org/node/2873849
+* @preserve
+**/
+
+(function ($, window, Drupal, drupalSettings) {
+ 'use strict';
+
+ Drupal.behaviors.insertTest = {
+ attach: function attach(context, settings) {
+ $('.ajax-insert').once('ajax-insert').on('click', function (event) {
+ event.preventDefault();
+ var ajaxSettings = {
+ url: event.currentTarget.getAttribute('href'),
+ wrapper: 'ajax-target',
+ base: false,
+ element: false,
+ method: event.currentTarget.getAttribute('data-method')
+ };
+ var myAjaxObject = Drupal.ajax(ajaxSettings);
+ myAjaxObject.execute();
+ });
+
+ $('.ajax-insert-inline').once('ajax-insert').on('click', function (event) {
+ event.preventDefault();
+ var ajaxSettings = {
+ url: event.currentTarget.getAttribute('href'),
+ wrapper: 'ajax-target-inline',
+ base: false,
+ element: false,
+ method: event.currentTarget.getAttribute('data-method')
+ };
+ var myAjaxObject = Drupal.ajax(ajaxSettings);
+ myAjaxObject.execute();
+ });
+
+ $(context).addClass('processed');
+ }
+ };
+})(jQuery, window, Drupal, drupalSettings);
\ No newline at end of file
diff --git a/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php
index 5ba65e8013..b0620cb256 100644
--- a/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php
+++ b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php
@@ -43,6 +43,61 @@ public static function dialogContents() {
}
/**
+ * Example content for testing whether response should be wrapped in div.
+ *
+ * @param string $type
+ * Type of response.
+ *
+ * @return array
+ * Renderable array of AJAX response contents.
+ */
+ public function renderTypes($type) {
+ $content = [
+ '#title' => 'AJAX Dialog & contents',
+ 'content' => [
+ '#type' => 'inline_template',
+ '#template' => $this->getRenderTypes()[$type],
+ ],
+ ];
+
+ return $content;
+ }
+
+ /**
+ * Returns a render array of links that directly Drupal.ajax().
+ */
+ public function insertLinks() {
+ $methods = [
+ 'html',
+ 'replaceWith',
+ ];
+
+ $build['links'] = [
+ 'ajax_target' => [
+ '#markup' => 'Target
Target inline',
+ ],
+ 'links' => [
+ '#theme' => 'links',
+ '#attached' => ['library' => ['ajax_test/ajax_insert']],
+ ],
+ ];
+ foreach ($methods as $method) {
+ foreach (array_keys($this->getRenderTypes()) as $type) {
+ $class = $type == 'inline' ? 'ajax-insert-inline' : 'ajax-insert';
+ $build['links']['links']['#links']["$method-$type"] = [
+ 'title' => "Link $method $type",
+ 'url' => Url::fromRoute('ajax_test.ajax_render_types', ['type' => $type]),
+ 'attributes' => [
+ 'class' => [$class],
+ 'data-method' => $method,
+ ],
+ ];
+ }
+ }
+ return $build;
+ }
+
+ /**
* Returns a render array that will be rendered by AjaxRenderer.
*
* Verifies that the response incorporates JavaScript settings generated
@@ -222,4 +277,26 @@ public function dialogClose() {
return $response;
}
+ /**
+ * Render types.
+ *
+ * @return array
+ * Render types.
+ */
+ protected function getRenderTypes() {
+ return [
+ 'pre-wrapped' => 'pre-wrapped
',
+ 'pre-wrapped-whitespace' => ' pre-wrapped-whitespace
' . "\r\n",
+ 'not-wrapped' => 'not-wrapped',
+ 'comment-not-wrapped' => '',
+ 'mixed' => ' foo foo bar additional not wrapped strings, final string
',
+ 'top-level-only' => 'element #1
element #2
',
+ 'top-level-only-pre-whitespace' => ' element #1
element #2
',
+ 'top-level-only-middle-whitespace' => 'element #1
element #2
',
+ 'inline' => 'inline BLOCK LEVEL
',
+ 'empty' => '',
+ ];
+ }
+
}
+
diff --git a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormPageCacheTest.php b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormPageCacheTest.php
index 3d174b04df..cf56c6241b 100644
--- a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormPageCacheTest.php
+++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormPageCacheTest.php
@@ -55,7 +55,7 @@ public function testSimpleAJAXFormValue() {
// Wait for the DOM to update. The HtmlCommand will update
// #ajax_selected_color to reflect the color change.
- $green_div = $this->assertSession()->waitForElement('css', "#ajax_selected_color div:contains('green')");
+ $green_div = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('green')");
$this->assertNotNull($green_div, 'DOM update: The selected color DIV is green.');
// Confirm the operation of the UpdateBuildIdCommand.
@@ -67,7 +67,7 @@ public function testSimpleAJAXFormValue() {
$session->getPage()->selectFieldOption('select', 'red');
// Wait for the DOM to update.
- $red_div = $this->assertSession()->waitForElement('css', "#ajax_selected_color div:contains('red')");
+ $red_div = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('red')");
$this->assertNotNull($red_div, 'DOM update: The selected color DIV is red.');
// Confirm the operation of the UpdateBuildIdCommand.
@@ -86,7 +86,7 @@ public function testSimpleAJAXFormValue() {
$session->getPage()->selectFieldOption('select', 'green');
// Wait for the DOM to update.
- $green_div2 = $this->assertSession()->waitForElement('css', "#ajax_selected_color div:contains('green')");
+ $green_div2 = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('green')");
$this->assertNotNull($green_div2, 'DOM update: After reload - the selected color DIV is green.');
$build_id_from_cache_first_ajax = $this->getFormBuildId();
@@ -98,7 +98,7 @@ public function testSimpleAJAXFormValue() {
$session->getPage()->selectFieldOption('select', 'red');
// Wait for the DOM to update.
- $red_div2 = $this->assertSession()->waitForElement('css', "#ajax_selected_color div:contains('red')");
+ $red_div2 = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('red')");
$this->assertNotNull($red_div2, 'DOM update: After reload - the selected color DIV is red.');
$build_id_from_cache_second_ajax = $this->getFormBuildId();
diff --git a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php
index e05940537c..8baa3f6c4f 100644
--- a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php
+++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php
@@ -16,6 +16,19 @@ class AjaxTest extends JavascriptTestBase {
*/
public static $modules = ['ajax_test'];
+ /**
+ * Wrap HTML with an AJAX target element.
+ *
+ * @param string $html
+ * The HTML to wrap.
+ *
+ * @return string
+ * The HTML wrapped in the an AJAX target element.
+ */
+ protected function wrapAjaxTarget($html) {
+ return 'data-drupal-ajax-target="">' . $html . '';
+ }
+
public function testAjaxWithAdminRoute() {
\Drupal::service('theme_installer')->install(['stable', 'seven']);
$theme_config = \Drupal::configFactory()->getEditable('system.theme');
@@ -82,4 +95,97 @@ public function testDrupalSettingsCachingRegression() {
$this->assertNotContains($fake_library, $libraries);
}
+ /**
+ * Tests that various AJAX responses with DOM elements are correctly inserted.
+ *
+ * After inserting DOM elements, Drupal JavaScript behaviors should be
+ * reattached and all top-level elements of type Node.ELEMENT_NODE need to be
+ * part of the context.
+ */
+ public function testInsert() {
+ $assert = $this->assertSession();
+ $test_cases = [
+ // Test that no additional wrapper is added when inserting already wrapped
+ // response data and all top-level node elements (context) are processed
+ // correctly.
+ [
+ 'render_type' => 'pre-wrapped',
+ 'expected' => 'pre-wrapped
',
+ ],
+ // Test that no additional empty leading div is added when the return
+ // value had a leading space and all top-level node elements (context) are
+ // processed correctly.
+ [
+ 'render_type' => 'pre-wrapped-whitespace',
+ 'expected' => 'pre-wrapped-whitespace
',
+ ],
+ // Test that not wrapped response data (text node) is inserted wrapped and
+ // all top-level node elements (context) are processed correctly.
+ [
+ 'render_type' => 'not-wrapped',
+ 'expected' => 'not-wrapped',
+ ],
+ // Test that top-level comments (which are not lead by text nodes) are
+ // inserted without wrapper.
+ [
+ 'render_type' => 'comment-not-wrapped',
+ 'expected' => '',
+ ],
+ // Test that wrappend and not-wrapped response data is inserted correctly
+ // and all top-level node elements (context) are processed correctly.
+ [
+ 'method' => 'html',
+ 'render_type' => 'mixed',
+ 'expected' => ' foo foo bar
additional not wrapped strings,
final string
',
+ ],
+ // Test that when the response has only top-level node elements, they
+ // are processed properly without extra wrapping.
+ [
+ 'method' => 'html',
+ 'render_type' => 'top-level-only',
+ 'expected' => 'element #1
element #2
',
+ ],
+ // Test that whitespaces at start or end don't wrap the response when
+ // there are multiple top-level nodes.
+ [
+ 'method' => 'html',
+ 'render_type' => 'top-level-only-pre-whitespace',
+ 'expected' => 'element #1
element #2
',
+ ],
+ // Test that when there are whitespaces between top-level nodes, the
+ // response is wrapped.
+ [
+ 'method' => 'html',
+ 'render_type' => 'top-level-only-middle-whitespace',
+ 'expected' => '',
+ ],
+ // Test that inline response data.
+ [
+ 'render_type' => 'inline',
+ 'expected' => 'inline BLOCK LEVEL
',
+ ],
+ // Test that empty response data.
+ [
+ 'render_type' => 'empty',
+ 'expected' => '',
+ ],
+ ];
+
+ $this->drupalGet('ajax-test/insert');
+ foreach ($test_cases as $test_case) {
+ $this->clickLink("Link html {$test_case['render_type']}");
+ $assert->assertWaitOnAjaxRequest();
+ // Extra span added by a second prepend command on the ajax requests.
+ $assert->responseContains($this->wrapAjaxTarget($test_case['expected']));
+ }
+
+ foreach ($test_cases as $test_case) {
+ $this->drupalGet('ajax-test/insert');
+ $this->clickLink("Link replaceWith {$test_case['render_type']}");
+ $assert->assertWaitOnAjaxRequest();
+ $assert->responseContains($test_case['expected']);
+ $assert->responseNotContains($this->wrapAjaxTarget($test_case['expected']));
+ }
+ }
+
}