diff --git a/core/misc/ajax.es6.js b/core/misc/ajax.es6.js index 6248e46..e96f3ae 100644 --- a/core/misc/ajax.es6.js +++ b/core/misc/ajax.es6.js @@ -1009,38 +1009,73 @@ else if (type === 'fade') { * A optional jQuery selector string. * @param {object} [response.settings] * An optional array of settings that will be used. - * @param {number} [status] - * The XMLHttpRequest status. */ - insert(ajax, response, status) { + insert(ajax, response) { // Get information from the response. If it is not there, default to // our presets. const $wrapper = response.selector ? $(response.selector) : $(ajax.wrapper); const method = response.method || ajax.method; const effect = ajax.getEffect(response); - let settings; + + // Apply any settings from the returned JSON if available. + const settings = response.settings || ajax.settings || drupalSettings; // We don't know what response.data contains: it might be a string of text // without HTML, so don't rely on jQuery correctly interpreting // $(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). - const $new_content_wrapped = $('
').html(response.data); - let $new_content = $new_content_wrapped.contents(); + // 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. + let $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 example: + // '
content
' is equivalent to '
content
' and + // no extra wrapper will be added. + const trimmedData = response.data.trim(); + // List of DOM nodes contained in the response for processing. + let responseDataNodes; - // 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; + // When 'data' is empty or is only whitespace, manually create the node + // array because parseHTML would return null. + if (trimmedData === '') { + responseDataNodes = [document.createTextNode(response.data)]; + } + else { + responseDataNodes = $.parseHTML(trimmedData, true); + } + // Check every node for it's type to decide if wrapping is necessary. + const onlyElementNodes = responseDataNodes.every( + element => element.nodeType === Node.ELEMENT_NODE || element.nodeType === Node.COMMENT_NODE, + ); + + // When there are only element and/or comment nodes in the response, no + // extra wrapping necessary. + if (onlyElementNodes) { + $newContent = $(trimmedData); + } + // When there are other types of top-level nodes, the response needs to be + // wrapped. + else { + const onlyTextOrCommentNodes = responseDataNodes.every( + element => element.nodeType === Node.COMMENT_NODE || element.nodeType === Node.TEXT_NODE, + ); + // If response.data contains only nodes of the type TEXT_NODE or + // COMMENT_NODE, or the wrapping element is not styled as a block, + // response.data will be wrapped with SPAN. + if (onlyTextOrCommentNodes || $wrapper.css('display') !== 'block') { + $newContent = $(''); + } + else { + $newContent = $('
'); + } + // Use response.data to keep whitespace as-is. + $newContent.html(response.data); } // If removing content from the wrapper, detach behaviors first. @@ -1050,36 +1085,42 @@ else if (type === 'fade') { case 'replaceAll': case 'empty': case 'remove': - settings = response.settings || ajax.settings || drupalSettings; Drupal.detachBehaviors($wrapper.get(0), settings); + break; + default: + break; } // 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); + const $ajaxNewContent = $newContent.find('.ajax-new-content'); + if ($ajaxNewContent.length) { + $ajaxNewContent.hide(); + $newContent.show(); + $ajaxNewContent[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) { - // Apply any settings from the returned JSON if available. - settings = response.settings || ajax.settings || drupalSettings; - Drupal.attachBehaviors($new_content.get(0), settings); + if ($newContent.parents('html').length) { + // Attach behaviors to all element nodes. + $newContent.each((index, element) => { + if (element.nodeType === Node.ELEMENT_NODE) { + Drupal.attachBehaviors(element, settings); + } + }); } }, diff --git a/core/misc/ajax.js b/core/misc/ajax.js index 5ea5242..c669d8f 100644 --- a/core/misc/ajax.js +++ b/core/misc/ajax.js @@ -474,46 +474,78 @@ function loadAjaxBehavior(base) { Drupal.AjaxCommands = function () {}; Drupal.AjaxCommands.prototype = { - insert: function insert(ajax, response, status) { + insert: function insert(ajax, response) { var $wrapper = response.selector ? $(response.selector) : $(ajax.wrapper); var method = response.method || ajax.method; var effect = ajax.getEffect(response); - var settings = void 0; - var $new_content_wrapped = $('
').html(response.data); - var $new_content = $new_content_wrapped.contents(); + var settings = response.settings || ajax.settings || drupalSettings; - if ($new_content.length !== 1 || $new_content.get(0).nodeType !== 1) { - $new_content = $new_content_wrapped; + var $newContent = void 0; + + var trimmedData = response.data.trim(); + + var responseDataNodes = void 0; + + if (trimmedData === '') { + responseDataNodes = [document.createTextNode(response.data)]; + } else { + responseDataNodes = $.parseHTML(trimmedData, true); } + var onlyElementNodes = responseDataNodes.every(function (element) { + return element.nodeType === Node.ELEMENT_NODE || element.nodeType === Node.COMMENT_NODE; + }); + + if (onlyElementNodes) { + $newContent = $(trimmedData); + } else { + var onlyTextOrCommentNodes = responseDataNodes.every(function (element) { + return element.nodeType === Node.COMMENT_NODE || element.nodeType === Node.TEXT_NODE; + }); + + if (onlyTextOrCommentNodes || $wrapper.css('display') !== 'block') { + $newContent = $(''); + } else { + $newContent = $('
'); + } + + $newContent.html(response.data); + } + switch (method) { case 'html': case 'replaceWith': case 'replaceAll': case 'empty': case 'remove': - settings = response.settings || ajax.settings || drupalSettings; Drupal.detachBehaviors($wrapper.get(0), settings); + break; + default: + break; } - $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); + var $ajaxNewContent = $newContent.find('.ajax-new-content'); + if ($ajaxNewContent.length) { + $ajaxNewContent.hide(); + $newContent.show(); + $ajaxNewContent[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) { - settings = response.settings || ajax.settings || drupalSettings; - Drupal.attachBehaviors($new_content.get(0), settings); + if ($newContent.parents('html').length) { + $newContent.each(function (index, element) { + if (element.nodeType === Node.ELEMENT_NODE) { + Drupal.attachBehaviors(element, settings); + } + }); } }, remove: function remove(ajax, response, status) { 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 f1c7306..772a05f 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 e8d06c0..89191a1 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 0000000..e7fb3f2 --- /dev/null +++ b/core/modules/system/tests/modules/ajax_test/js/insert-ajax.es6.js @@ -0,0 +1,39 @@ +/** + * @file + * Drupal behavior to attach click event handlers to ajax-insert and + * ajax-insert-inline links for testing ajax requests. + */ + +(function ($, window, Drupal) { + Drupal.behaviors.insertTest = { + attach(context) { + $('.ajax-insert').once('ajax-insert').on('click', (event) => { + event.preventDefault(); + const ajaxSettings = { + url: event.currentTarget.getAttribute('href'), + wrapper: 'ajax-target', + base: false, + element: false, + method: event.currentTarget.getAttribute('data-method'), + }; + const myAjaxObject = Drupal.ajax(ajaxSettings); + myAjaxObject.execute(); + }); + + $('.ajax-insert-inline').once('ajax-insert').on('click', (event) => { + event.preventDefault(); + const ajaxSettings = { + url: event.currentTarget.getAttribute('href'), + wrapper: 'ajax-target-inline', + base: false, + element: false, + method: event.currentTarget.getAttribute('data-method'), + }; + const myAjaxObject = Drupal.ajax(ajaxSettings); + myAjaxObject.execute(); + }); + + $(context).addClass('processed'); + }, + }; +}(jQuery, window, Drupal)); 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 0000000..c261bca --- /dev/null +++ b/core/modules/system/tests/modules/ajax_test/js/insert-ajax.js @@ -0,0 +1,40 @@ +/** +* DO NOT EDIT THIS FILE. +* See the following change record for more information, +* https://www.drupal.org/node/2815083 +* @preserve +**/ + +(function ($, window, Drupal) { + Drupal.behaviors.insertTest = { + attach: function attach(context) { + $('.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); \ 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 5ba65e8..fcffcd1 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,27 @@ 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-string-not-wrapped' => 'comment-string-not-wrapped', + 'comment-not-wrapped' => '
comment-not-wrapped
', + 'mixed' => ' foo foo bar

some string

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 3d174b0..afa8420 100644 --- a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormPageCacheTest.php +++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormPageCacheTest.php @@ -55,8 +55,8 @@ 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')"); - $this->assertNotNull($green_div, 'DOM update: The selected color DIV is green.'); + $green_span = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('green')"); + $this->assertNotNull($green_span, 'DOM update: The selected color SPAN is green.'); // Confirm the operation of the UpdateBuildIdCommand. $build_id_first_ajax = $this->getFormBuildId(); @@ -67,8 +67,8 @@ 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')"); - $this->assertNotNull($red_div, 'DOM update: The selected color DIV is red.'); + $red_span = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('red')"); + $this->assertNotNull($red_span, 'DOM update: The selected color SPAN is red.'); // Confirm the operation of the UpdateBuildIdCommand. $build_id_second_ajax = $this->getFormBuildId(); @@ -86,8 +86,8 @@ 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')"); - $this->assertNotNull($green_div2, 'DOM update: After reload - the selected color DIV is green.'); + $green_span2 = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('green')"); + $this->assertNotNull($green_span2, 'DOM update: After reload - the selected color SPAN is green.'); $build_id_from_cache_first_ajax = $this->getFormBuildId(); $this->assertNotEquals($build_id_from_cache_initial, $build_id_from_cache_first_ajax, 'Build id is changed in the simpletest-DOM on first AJAX submission'); @@ -98,8 +98,8 @@ 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')"); - $this->assertNotNull($red_div2, 'DOM update: After reload - the selected color DIV is red.'); + $red_span2 = $this->assertSession()->waitForElement('css', "#ajax_selected_color span:contains('red')"); + $this->assertNotNull($red_span2, 'DOM update: After reload - the selected color SPAN is red.'); $build_id_from_cache_second_ajax = $this->getFormBuildId(); $this->assertNotEquals($build_id_from_cache_first_ajax, $build_id_from_cache_second_ajax, 'Build id changes on subsequent AJAX submissions'); diff --git a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php index e059405..3633b65 100644 --- a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php +++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php @@ -16,6 +16,25 @@ class AjaxTest extends JavascriptTestBase { */ public static $modules = ['ajax_test']; + /** + * Wraps HTML with an AJAX target element. + * + * This reproduces recognizable parts of the wrapping markup from + * \Drupal\ajax_test\Controller\AjaxTestController::insertLinks and is not + * supposed to return valid HTML. + * + * @param string $html + * The HTML to wrap. + * + * @return string + * The HTML wrapped in the an AJAX target element. + * + * @see \Drupal\ajax_test\Controller\AjaxTestController::insertLinks + */ + protected function wrapAjaxTarget($html) { + return 'data-drupal-ajax-target="">' . $html . 'install(['stable', 'seven']); $theme_config = \Drupal::configFactory()->getEditable('system.theme'); @@ -82,4 +101,105 @@ 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. + * + * @dataProvider providerTestInsert + */ + public function testInsert($render_type, $expected) { + $assert = $this->assertSession(); + + $this->drupalGet('ajax-test/insert'); + $this->clickLink("Link html $render_type"); + $assert->assertWaitOnAjaxRequest(); + // Extra span added by a second prepend command on the ajax requests. + $assert->responseContains($this->wrapAjaxTarget($expected)); + + $this->drupalGet('ajax-test/insert'); + $this->clickLink("Link replaceWith $render_type"); + $assert->assertWaitOnAjaxRequest(); + $assert->responseContains($expected); + $assert->responseNotContains($this->wrapAjaxTarget($expected)); + } + + /** + * Provides test data for ::testInsert(). + */ + public function providerTestInsert() { + $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. + $test_cases['pre_wrapped'] = [ + 'pre-wrapped', + '
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. + $test_cases['pre_wrapped_whitespace'] = [ + 'pre-wrapped-whitespace', + '
pre-wrapped-whitespace
', + ]; + // Test that not wrapped response data (text node) is inserted wrapped and + // all top-level node elements (context) are processed correctly. + $test_cases['not_wrapped'] = [ + 'not-wrapped', + 'not-wrapped', + ]; + // Test that not wrapped response data (text node and comment node) is + // inserted wrapped and all top-level node elements + // (context) are processed correctly. + $test_cases['comment_string_not_wrapped'] = [ + 'comment-string-not-wrapped', + 'comment-string-not-wrapped', + ]; + // Test that top-level comments (which are not lead by text nodes) are + // inserted without wrapper. + $test_cases['comment_not_wrapped'] = [ + 'comment-not-wrapped', + '
comment-not-wrapped
', + ]; + // Test that wrappend and not-wrapped response data is inserted correctly + // and all top-level node elements (context) are processed correctly. + $test_cases['mixed'] = [ + 'mixed', + '
foo foo bar

some string

additional not wrapped strings,

final string

', + ]; + // Test that when the response has only top-level node elements, they + // are processed properly without extra wrapping. + $test_cases['top_level_only'] = [ + 'top-level-only', + '
element #1
element #2
', + ]; + // Test that whitespaces at start or end don't wrap the response when + // there are multiple top-level nodes. + $test_cases['top_level_only_pre_whitespace'] = [ + 'top-level-only-pre-whitespace', + '
element #1
element #2
', + ]; + // Test that when there are whitespaces between top-level nodes, the + // response is wrapped. + $test_cases['top_level_only_middle_whitespace'] = [ + 'top-level-only-middle-whitespace', + '
element #1
element #2
', + ]; + // Test that inline response data. + $test_cases['inline'] = [ + 'inline', + 'inline
BLOCK LEVEL
', + ]; + // Test that empty response data. + $test_cases['empty'] = [ + 'empty', + '', + ]; + return $test_cases; + } + }