core/lib/Drupal/Core/Render/Renderer.php | 34 ++++++- .../FunctionalJavascript/BigPipeRegressionTest.php | 56 ++++++++++-- .../render_placeholder_message_test.info.yml | 6 ++ .../render_placeholder_message_test.routing.yml | 24 +++++ .../src/RenderPlaceholderMessageTestController.php | 100 +++++++++++++++++++++ .../Functional/Render/PlaceholderMessageTest.php | 58 ++++++++++++ 6 files changed, 271 insertions(+), 7 deletions(-) diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index 5209eb5..4e98db7 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/tests/src/FunctionalJavascript/BigPipeRegressionTest.php b/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php index ab28073..addda90 100644 --- a/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php +++ b/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php @@ -6,6 +6,7 @@ use Drupal\comment\Entity\Comment; use Drupal\comment\Plugin\Field\FieldType\CommentItemInterface; use Drupal\comment\Tests\CommentTestTrait; +use Drupal\Core\Url; use Drupal\editor\Entity\Editor; use Drupal\filter\Entity\FilterFormat; use Drupal\FunctionalJavascriptTests\JavascriptTestBase; @@ -27,13 +28,7 @@ class BigPipeRegressionTest extends JavascriptTestBase { * {@inheritdoc} */ public static $modules = [ - 'node', - 'comment', 'big_pipe', - 'history', - 'editor', - 'ckeditor', - 'filter', ]; /** @@ -53,6 +48,14 @@ public function setUp() { * @see https://www.drupal.org/node/2698811 */ public function testCommentForm_2698811() { + $this->assertTrue($this->container->get('module_installer')->install([ 'node', + 'comment', + 'history', + 'editor', + 'ckeditor', + 'filter', + ], TRUE), 'Installed modules.'); + // Ensure an `article` node type exists. $this->createContentType(['type' => 'article']); $this->addDefaultCommentField('node', 'article'); @@ -114,4 +117,45 @@ public function testCommentForm_2698811() { $this->assertJsCondition($javascript); } + /** + * Ensure BigPipe renders the "messages" placeholder last. + * + * @see https://www.drupal.org/node/2712935 + */ + public function testMessages_2712935() { + $this->assertTrue($this->container->get('module_installer')->install(['render_placeholder_message_test'], TRUE), 'Installed modules.'); + + $this->drupalLogin($this->drupalCreateUser()); + $messages_markup = '
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..7aeaadb --- /dev/null +++ b/core/modules/system/tests/modules/render_placeholder_message_test/src/RenderPlaceholderMessageTestController.php @@ -0,0 +1,100 @@ +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); + } + } + +}