diff -u b/core/misc/ajax.js b/core/misc/ajax.js --- b/core/misc/ajax.js +++ b/core/misc/ajax.js @@ -1027,55 +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). - - // 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. + // 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 $responseDataWrapped = $('
').html(response.data); var $new_content = $('
'); var elementsReturned = $responseDataWrapped.contents().length; - var createWrapper = function () { - return $('
'); - }; - - var intermediateWrapper = null; - $responseDataWrapped.contents().each(function (index, value) { - if (intermediateWrapper && value.nodeType !== 1) { - intermediateWrapper.append(value); - } - if (!intermediateWrapper && value.nodeType !== 1) { - if ($.trim(value.nodeValue).length) { - intermediateWrapper = createWrapper(); - intermediateWrapper.append(value); - } - } - if (!intermediateWrapper && value.nodeType === 1) { - $new_content.append(value); - intermediateWrapper = null; - } - if (intermediateWrapper && value.nodeType === 1) { - $new_content.append(intermediateWrapper, value); - intermediateWrapper = null; - } - }); - - // If we only had one element returned, and not an Node.ELEMENT_NODE - // we need to add it to $new_content - if (elementsReturned === 1) { - $new_content.append(intermediateWrapper); + // If only one node element is returned skip wrap processing. + if (elementsReturned === 1 && response.data === Node.ELEMENT_NODE) { + $new_content = response.data; } + else { + var $intermediateWrapper = null; + + $responseDataWrapped.contents().each(function (index, value) { + // Make sure all not-element nodes are wrapped. + if (value.nodeType !== Node.ELEMENT_NODE) { + if ($intermediateWrapper) { + $intermediateWrapper.append(value); + } + // Make an exception for comments that are not after a text node. + // This is especially helpful for when theme debugging is turned on. + else if (value.nodeType === Node.COMMENT_NODE) { + $new_content.append(value); + } + // Only create a wrapper if there is more than whitespace. + else if ($.trim(value.nodeValue).length) { + $intermediateWrapper = $(''); + $intermediateWrapper.append(value); + } + } + else { + $new_content.append($intermediateWrapper, value); + $intermediateWrapper = null; + } + + // There are no other elements to process anymore, so add the + // intermediate wrapper to the content if present. + if (index === elementsReturned - 1 && $intermediateWrapper) { + $new_content.append($intermediateWrapper); + } + }); - $new_content = $new_content.children(); + $new_content = $new_content.contents(); + } // If removing content from the wrapper, detach behaviors first. switch (method) { @@ -1085,7 +1083,10 @@ case 'empty': case 'remove': settings = response.settings || ajax.settings || drupalSettings; - Drupal.detachBehaviors($wrapper.get(0), settings); + // Detach behaviors of all the to be removed element nodes. + $wrapper.children().each(function () { + Drupal.detachBehaviors(this, settings); + }); } // Add the new content to the page. @@ -1113,7 +1114,10 @@ if ($new_content.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. + $wrapper.children().each(function () { + Drupal.attachBehaviors(this, settings); + }); } }, diff -u b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php --- b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php +++ b/core/modules/system/tests/modules/ajax_test/src/Controller/AjaxTestController.php @@ -65,8 +65,12 @@ $markup = $type; break; + case 'comment-not-wrapped': + $markup = '
' . $type . '
'; + break; + case 'mixed': - $markup = ' foo foo bar

some string

additional wrapped strings,

final string

'; + $markup = ' foo foo bar

some string

additional not wrapped strings,

final string

'; break; } @@ -89,6 +93,7 @@ 'pre-wrapped', 'pre-wrapped-leading-whitespace', 'not-wrapped', + 'comment-not-wrapped', 'mixed', ]; diff -u b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php --- b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php +++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php @@ -16,6 +16,19 @@ */ public static $modules = ['ajax_test']; + /** + * Wrap HTML with an AJAX target div. + * + * @param string $html + * The HTML to wrap. + * + * @return string + * The HTML wrapped in the an AJAX target div. + */ + protected function wrapAjaxTarget($html) { + return '
' . $html . '
'; + } + public function testAjaxWithAdminRoute() { \Drupal::service('theme_installer')->install(['stable', 'seven']); $theme_config = \Drupal::configFactory()->getEditable('system.theme'); @@ -86,7 +99,7 @@ * Tests that various AJAX responses with DOM elements are correctly inserted. * * After inserting DOM elements, Drupal JavaScript behaviors should be - * reattached and all 'root' elements of type Node.ELEMENT_NODE need to be + * reattached and all top-level elements of type Node.ELEMENT_NODE need to be * part of the context. */ public function testInsert() { @@ -94,30 +107,36 @@ $this->drupalGet('ajax-test/insert'); // Test that no additional wrapper is added when inserting already wrapped - // response data and all root node elements (context) are processed + // response data and all top-level node elements (context) are processed // correctly. $this->clickLink('Link pre-wrapped'); $assert->assertWaitOnAjaxRequest(); - $assert->responseContains('
pre-wrapped
'); + $assert->responseContains($this->wrapAjaxTarget('
pre-wrapped
')); // Test that no additional empty leading div is added when the return - // value had a leading space and all root node elements (context) are + // value had a leading space and all top-level node elements (context) are // processed correctly. $this->clickLink('Link pre-wrapped-leading-whitespace'); $assert->assertWaitOnAjaxRequest(); - $assert->responseContains('
pre-wrapped-leading-whitespace
'); + $assert->responseContains($this->wrapAjaxTarget('
pre-wrapped-leading-whitespace
')); - // Test that unwrapped response data (text node) is inserted wrapped and all - // root node elements (context) are processed correctly. + // Test that not wrapped response data (text node) is inserted wrapped and + // all top-level node elements (context) are processed correctly. $this->clickLink('Link not-wrapped'); $assert->assertWaitOnAjaxRequest(); - $assert->responseContains('
not-wrapped
'); + $assert->responseContains($this->wrapAjaxTarget('not-wrapped')); + + // Test that top-level comments (which are not lead by text nodes) are + // inserted without wrapper. + $this->clickLink('Link comment-not-wrapped'); + $assert->assertWaitOnAjaxRequest(); + $assert->responseContains($this->wrapAjaxTarget('
comment-not-wrapped
')); - // Test that wrappend and unwrapped response data is inserted correctly and - // all root node elements (context) are processed correctly. + // Test that wrappend and not-wrapped response data is inserted correctly + // and all top-level node elements (context) are processed correctly. $this->clickLink('Link mixed'); $assert->assertWaitOnAjaxRequest(); - $assert->responseContains('
foo foo bar

some string

additional wrapped strings,

final string

'); + $assert->responseContains($this->wrapAjaxTarget(' foo foo bar

some string

additional not wrapped strings,

final string

')); } }