core/lib/Drupal/Core/Render/Renderer.php | 34 +++++++- core/modules/big_pipe/src/Render/BigPipe.php | 33 +++++++- core/modules/big_pipe/src/Tests/BigPipeTest.php | 18 +++- .../FunctionalJavascript/BigPipeRegressionTest.php | 50 +++++++++-- .../render_placeholder_message_test.info.yml | 6 ++ .../render_placeholder_message_test.routing.yml | 24 ++++++ .../src/RenderPlaceholderMessageTestController.php | 99 ++++++++++++++++++++++ .../Functional/Render/PlaceholderMessageTest.php | 58 +++++++++++++ 8 files changed, 309 insertions(+), 13 deletions(-) diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index 5209eb5..a624c0f 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -653,7 +653,39 @@ protected function replacePlaceholders(array &$elements) { return FALSE; } - foreach (array_keys($elements['#attached']['placeholders']) as $placeholder) { + // The 'status messages' placeholder needs to be special cased, because it + // depends on global state that can be modified when other placeholders are + // being rendered: any code can add messages to render. + // This violates the principle that each lazy builder must be able to render + // itself in isolation, and therefore in any order. However, we cannot + // change the way drupal_set_message() works in the Drupal 8 cycle. So we + // have to accommodate its special needs. + // Allowing placeholders to be rendered in a particular order (in this case: + // last) would violate this isolation principle. Thus a monopoly is granted + // to this one special case, with this hard-coded solution. + // @see \Drupal\Core\Render\Element\StatusMessages + // @see https://www.drupal.org/node/2712935#comment-11368923 + $possible_message_placeholders = [ + '', + '', + '', + '', + ]; + + // We hardcoded the placeholders above to avoid doing this on every request, + // but we ensure those hardcoded strings are accurate via assertions. + assert('$possible_message_placeholders[0] === (string) $this->placeholderGenerator->createPlaceholder(["#lazy_builder" => ["Drupal\\Core\\Render\\Element\\StatusMessages::renderMessages", [NULL]], "#create_placeholder" => TRUE])["#markup"]'); + assert('$possible_message_placeholders[1] === (string) $this->placeholderGenerator->createPlaceholder(["#lazy_builder" => ["Drupal\\Core\\Render\\Element\\StatusMessages::renderMessages", ["status"]], "#create_placeholder" => TRUE])["#markup"]'); + assert('$possible_message_placeholders[2] === (string) $this->placeholderGenerator->createPlaceholder(["#lazy_builder" => ["Drupal\\Core\\Render\\Element\\StatusMessages::renderMessages", ["warning"]], "#create_placeholder" => TRUE])["#markup"]'); + assert('$possible_message_placeholders[3] === (string) $this->placeholderGenerator->createPlaceholder(["#lazy_builder" => ["Drupal\\Core\\Render\\Element\\StatusMessages::renderMessages", ["error"]], "#create_placeholder" => TRUE])["#markup"]'); + + // Render placeholders in the appropriate order. + $placeholders = array_keys($elements['#attached']['placeholders']); + $ordered_placeholders = array_merge( + array_diff($placeholders, $possible_message_placeholders), + array_intersect($placeholders, $possible_message_placeholders) + ); + foreach ($ordered_placeholders as $placeholder) { $elements = $this->renderPlaceholder($placeholder, $elements); } diff --git a/core/modules/big_pipe/src/Render/BigPipe.php b/core/modules/big_pipe/src/Render/BigPipe.php index 5e3cc12..00ec4a2 100644 --- a/core/modules/big_pipe/src/Render/BigPipe.php +++ b/core/modules/big_pipe/src/Render/BigPipe.php @@ -538,15 +538,42 @@ protected function renderPlaceholder($placeholder, array $placeholder_render_arr protected function getPlaceholderOrder($html) { $fragments = explode('
', $fragment, 2); $placeholder = $t[0]; - $order[] = $placeholder; + $placeholders[] = $placeholder; } + $placeholders = array_unique($placeholders); + + // The 'status messages' placeholder needs to be special cased, because it + // depends on global state that can be modified when other placeholders are + // being rendered: any code can add messages to render. + // This violates the principle that each lazy builder must be able to render + // itself in isolation, and therefore in any order. However, we cannot + // change the way drupal_set_message() works in the Drupal 8 cycle. So we + // have to accommodate its special needs. + // Allowing placeholders to be rendered in a particular order (in this case: + // last) would violate this isolation principle. Thus a monopoly is granted + // to this one special case, with this hard-coded solution. + // @see \Drupal\Core\Render\Element\StatusMessages + // @see \Drupal\Core\Render\Renderer::replacePlaceholders() + // @see https://www.drupal.org/node/2712935#comment-11368923 + $possible_message_placeholders = [ + 'callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args[0]&token=a8c34b5e', + 'callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args[0]=status&token=a26f536e', + 'callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args[0]=warning&token=67320bbd', + 'callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args[0]=error&token=35b549cb', + ]; - return array_unique($order); + // Return placeholder IDs in DOM order, but with the 'status messages' at + // the end, if it exists. + $ordered_placeholders = array_merge( + array_diff($placeholders, $possible_message_placeholders), + array_intersect($placeholders, $possible_message_placeholders) + ); + return $ordered_placeholders; } } diff --git a/core/modules/big_pipe/src/Tests/BigPipeTest.php b/core/modules/big_pipe/src/Tests/BigPipeTest.php index 370b815..1a1bc3d 100644 --- a/core/modules/big_pipe/src/Tests/BigPipeTest.php +++ b/core/modules/big_pipe/src/Tests/BigPipeTest.php @@ -170,6 +170,11 @@ public function testBigPipe() { $cases['edge_case__html_non_lazy_builder']->bigPipePlaceholderId => Json::encode($cases['edge_case__html_non_lazy_builder']->embeddedAjaxResponseCommands), $cases['exception__lazy_builder']->bigPipePlaceholderId => NULL, $cases['exception__embedded_response']->bigPipePlaceholderId => NULL, + ], [ + 0 => $cases['edge_case__html_non_lazy_builder']->bigPipePlaceholderId, + // The 'html' case contains the 'status messages' placeholder, which is + // always rendered last. + 1 => $cases['html']->bigPipePlaceholderId, ]); $this->assertRaw('', 'Closing body tag present.'); @@ -336,8 +341,11 @@ protected function assertBigPipeNoJsPlaceholders(array $expected_big_pipe_nojs_p * * @param array $expected_big_pipe_placeholders * Keys: BigPipe placeholder IDs. Values: expected AJAX response. + * @param array $expected_big_pipe_placeholder_stream_order + * Keys: BigPipe placeholder IDs. Values: expected AJAX response. Keys are + * defined in the order that they are expected to be rendered & streamed. */ - protected function assertBigPipePlaceholders(array $expected_big_pipe_placeholders) { + protected function assertBigPipePlaceholders(array $expected_big_pipe_placeholders, array $expected_big_pipe_placeholder_stream_order) { $this->pass('Verifying BigPipe placeholders & replacements…', 'Debug'); $this->assertSetsEqual(array_keys($expected_big_pipe_placeholders), explode(' ', $this->drupalGetHeader('BigPipe-Test-Placeholders'))); $placeholder_positions = []; @@ -365,8 +373,12 @@ protected function assertBigPipePlaceholders(array $expected_big_pipe_placeholde ksort($placeholder_positions, SORT_NUMERIC); $this->assertEqual(array_keys($expected_big_pipe_placeholders), array_values($placeholder_positions)); $this->assertEqual(count($expected_big_pipe_placeholders), preg_match_all('/' . preg_quote('
assertSession(); + foreach ($test_routes as $route) { + // Verify that we start off with zero messages queued. + $this->drupalGet(Url::fromRoute('render_placeholder_message_test.queued')); + $assert->responseNotContains($messages_markup); + + // Verify the test case at this route behaves as expected. + $this->drupalGet(Url::fromRoute($route)); + $assert->elementContains('css', 'p.logged-message:nth-of-type(1)', 'Message: P1'); + $assert->elementContains('css', 'p.logged-message:nth-of-type(2)', 'Message: P2'); + $assert->responseContains($messages_markup); + $assert->elementExists('css', 'div[aria-label="Status message"] ul'); + $assert->elementContains('css', 'div[aria-label="Status message"] ul li:nth-of-type(1)', 'P1'); + $assert->elementContains('css', 'div[aria-label="Status message"] ul li:nth-of-type(2)', 'P2'); + + // Verify that we end with all messages printed, hence again zero queued. + $this->drupalGet(Url::fromRoute('render_placeholder_message_test.queued')); + $assert->responseNotContains($messages_markup); + } + } + } diff --git a/core/modules/system/tests/modules/render_placeholder_message_test/render_placeholder_message_test.info.yml b/core/modules/system/tests/modules/render_placeholder_message_test/render_placeholder_message_test.info.yml new file mode 100644 index 0000000..532695b --- /dev/null +++ b/core/modules/system/tests/modules/render_placeholder_message_test/render_placeholder_message_test.info.yml @@ -0,0 +1,6 @@ +name: 'Placeholder setting a message test' +type: module +description: 'Support module for PlaceholderMessageTest.' +package: Testing +version: VERSION +core: 8.x diff --git a/core/modules/system/tests/modules/render_placeholder_message_test/render_placeholder_message_test.routing.yml b/core/modules/system/tests/modules/render_placeholder_message_test/render_placeholder_message_test.routing.yml new file mode 100644 index 0000000..6734cf3 --- /dev/null +++ b/core/modules/system/tests/modules/render_placeholder_message_test/render_placeholder_message_test.routing.yml @@ -0,0 +1,24 @@ +render_placeholder_message_test.first: + path: '/render_placeholder_message_test/first' + defaults: + _controller: '\Drupal\render_placeholder_message_test\RenderPlaceholderMessageTestController::messagesPlaceholderFirst' + requirements: + _access: 'TRUE' +render_placeholder_message_test.middle: + path: '/render_placeholder_message_test/middle' + defaults: + _controller: '\Drupal\render_placeholder_message_test\RenderPlaceholderMessageTestController::messagesPlaceholderMiddle' + requirements: + _access: 'TRUE' +render_placeholder_message_test.last: + path: '/render_placeholder_message_test/last' + defaults: + _controller: '\Drupal\render_placeholder_message_test\RenderPlaceholderMessageTestController::messagesPlaceholderLast' + requirements: + _access: 'TRUE' +render_placeholder_message_test.queued: + path: '/render_placeholder_message_test/queued' + defaults: + _controller: '\Drupal\render_placeholder_message_test\RenderPlaceholderMessageTestController::queuedMessages' + requirements: + _access: 'TRUE' diff --git a/core/modules/system/tests/modules/render_placeholder_message_test/src/RenderPlaceholderMessageTestController.php b/core/modules/system/tests/modules/render_placeholder_message_test/src/RenderPlaceholderMessageTestController.php new file mode 100644 index 0000000..f02f0b6 --- /dev/null +++ b/core/modules/system/tests/modules/render_placeholder_message_test/src/RenderPlaceholderMessageTestController.php @@ -0,0 +1,99 @@ +build([ + '', + '', + '', + ]); + } + + /** + * @return array + */ + public function messagesPlaceholderMiddle() { + return $this->build([ + '', + '', + '', + ]); + } + + /** + * @return array + */ + public function messagesPlaceholderLast() { + return $this->build([ + '', + '', + '', + ]); + } + + /** + * @return array + */ + public function queuedMessages() { + return ['#type' => 'status_messages']; + } + + /** + * @return array + */ + protected function build(array $placeholder_order) { + $build = []; + $build['messages'] = ['#type' => 'status_messages']; + $build['p1'] = [ + '#lazy_builder' => ['\Drupal\render_placeholder_message_test\RenderPlaceholderMessageTestController::setAndLogMessage', ['P1']], + '#create_placeholder' => TRUE, + ]; + $build['p2'] = [ + '#lazy_builder' => ['\Drupal\render_placeholder_message_test\RenderPlaceholderMessageTestController::setAndLogMessage', ['P2']], + '#create_placeholder' => TRUE, + ]; + + /** @var \Drupal\Core\Render\RendererInterface $renderer */ + $renderer = $this->container->get('renderer'); + $renderer->executeInRenderContext(new RenderContext(), function () use (&$build, $renderer) { + return $renderer->render($build, FALSE); + }); + + $reordered = []; + foreach ($placeholder_order as $placeholder) { + $reordered[$placeholder] = $build['#attached']['placeholders'][$placeholder]; + } + $build['#attached']['placeholders'] = $reordered; + + return $build; + } + + /** + * #lazy_builder callback; sets and prints a message. + * + * @param string $message + * The message to send. + * + * @return array + * A renderable array containing the message. + */ + public static function setAndLogMessage($message) { + // Set message. + drupal_set_message($message); + + // Print which message is expected. + return ['#markup' => '

Message: ' . $message . '

']; + } + +} diff --git a/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php b/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php new file mode 100644 index 0000000..185ca49 --- /dev/null +++ b/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php @@ -0,0 +1,58 @@ +assertSession(); + foreach ($test_routes as $route) { + // Verify that we start off with zero messages queued. + $this->drupalGet(Url::fromRoute('render_placeholder_message_test.queued')); + $assert->responseNotContains($messages_markup); + + // Verify the test case at this route behaves as expected. + $this->drupalGet(Url::fromRoute($route)); + $assert->elementContains('css', 'p.logged-message:nth-of-type(1)', 'Message: P1'); + $assert->elementContains('css', 'p.logged-message:nth-of-type(2)', 'Message: P2'); + $assert->responseContains($messages_markup); + $assert->elementExists('css', 'div[aria-label="Status message"] ul'); + $assert->elementContains('css', 'div[aria-label="Status message"] ul li:nth-of-type(1)', 'P1'); + $assert->elementContains('css', 'div[aria-label="Status message"] ul li:nth-of-type(2)', 'P2'); + + // Verify that we end with all messages printed, hence again zero queued. + $this->drupalGet(Url::fromRoute('render_placeholder_message_test.queued')); + $assert->responseNotContains($messages_markup); + } + } + +}