diff --git a/core/modules/big_pipe/big_pipe.services.yml b/core/modules/big_pipe/big_pipe.services.yml index 4f653a1357e42e5d8d588b042f213c75cb736fc9..e1080f084dce0053d127c9fed5dbfe33b0bce024 100644 --- a/core/modules/big_pipe/big_pipe.services.yml +++ b/core/modules/big_pipe/big_pipe.services.yml @@ -11,7 +11,7 @@ services: - { name: placeholder_strategy, priority: 0 } big_pipe: class: Drupal\big_pipe\Render\BigPipe - arguments: ['@renderer', '@session', '@request_stack', '@http_kernel', '@event_dispatcher', '@config.factory'] + arguments: ['@renderer', '@session', '@request_stack', '@http_kernel', '@event_dispatcher', '@config.factory', '@messenger'] Drupal\big_pipe\Render\BigPipe: '@big_pipe' html_response.attachments_processor.big_pipe: public: false diff --git a/core/modules/big_pipe/src/Render/BigPipe.php b/core/modules/big_pipe/src/Render/BigPipe.php index 4f479d663adb02093ffef9fddad6c4077aaf0c19..1dab69c4e3c0a0df900b6d235d817dcc4d3b6d87 100644 --- a/core/modules/big_pipe/src/Render/BigPipe.php +++ b/core/modules/big_pipe/src/Render/BigPipe.php @@ -5,10 +5,12 @@ use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Html; use Drupal\Core\Ajax\AjaxResponse; +use Drupal\Core\Ajax\MessageCommand; use Drupal\Core\Ajax\ReplaceCommand; use Drupal\Core\Asset\AttachedAssets; use Drupal\Core\Asset\AttachedAssetsInterface; use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\Render\HtmlResponse; use Drupal\Core\Render\RendererInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -205,29 +207,17 @@ class BigPipe { */ protected $configFactory; - /** - * Constructs a new BigPipe class. - * - * @param \Drupal\Core\Render\RendererInterface $renderer - * The renderer. - * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session - * The session. - * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack - * The request stack. - * @param \Symfony\Component\HttpKernel\HttpKernelInterface $http_kernel - * The HTTP kernel. - * @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $event_dispatcher - * The event dispatcher. - * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory - * The config factory. - */ - public function __construct(RendererInterface $renderer, SessionInterface $session, RequestStack $request_stack, HttpKernelInterface $http_kernel, EventDispatcherInterface $event_dispatcher, ConfigFactoryInterface $config_factory) { + public function __construct(RendererInterface $renderer, SessionInterface $session, RequestStack $request_stack, HttpKernelInterface $http_kernel, EventDispatcherInterface $event_dispatcher, ConfigFactoryInterface $config_factory, protected ?MessengerInterface $messenger = NULL) { $this->renderer = $renderer; $this->session = $session; $this->requestStack = $request_stack; $this->httpKernel = $http_kernel; $this->eventDispatcher = $event_dispatcher; $this->configFactory = $config_factory; + if (!isset($this->messenger)) { + @trigger_error('Calling ' . __CLASS__ . '::_construct() without the $messenger argument is deprecated in drupal:10.3.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/3343754', E_USER_DEPRECATED); + $this->messenger = \Drupal::messenger(); + } } /** @@ -538,29 +528,16 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde // Create a Fiber for each placeholder. $fibers = []; - $message_placeholder_id = NULL; foreach ($placeholder_order as $placeholder_id) { if (!isset($placeholders[$placeholder_id])) { continue; } $placeholder_render_array = $placeholders[$placeholder_id]; - - // Ensure the messages placeholder renders last, the render order of every - // other placeholder is safe to change. - // @see static::getPlaceholderOrder() - if (isset($placeholder_render_array['#lazy_builder']) && $placeholder_render_array['#lazy_builder'][0] === 'Drupal\Core\Render\Element\StatusMessages::renderMessages') { - $message_placeholder_id = $placeholder_id; - } $fibers[$placeholder_id] = new \Fiber(fn() => $this->renderPlaceholder($placeholder_id, $placeholder_render_array)); } $iterations = 0; while (count($fibers) > 0) { foreach ($fibers as $placeholder_id => $fiber) { - // Keep skipping the messages placeholder until it's the only Fiber - // remaining. @todo https://www.drupal.org/project/drupal/issues/3379885 - if (isset($message_placeholder_id) && $placeholder_id === $message_placeholder_id && count($fibers) > 1) { - continue; - } try { if (!$fiber->isStarted()) { $fiber->start(); @@ -594,6 +571,15 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde $ajax_response->addCommand(new ReplaceCommand(sprintf('[data-big-pipe-placeholder-id="%s"]', $big_pipe_js_placeholder_id), $elements['#markup'])); $ajax_response->setAttachments($elements['#attached']); + // Delete all messages that were generated during the rendering of this + // placeholder, to render them in a BigPipe-optimized way. + $messages = $this->messenger->deleteAll(); + foreach ($messages as $type => $type_messages) { + foreach ($type_messages as $message) { + $ajax_response->addCommand(new MessageCommand($message, NULL, ['type' => $type], FALSE)); + } + } + // Push a fake request with the asset libraries loaded so far and // dispatch KernelEvents::RESPONSE event. This results in the // attachments for the AJAX response being processed by @@ -736,8 +722,7 @@ protected function renderPlaceholder($placeholder, array $placeholder_render_arr * * @return array * Indexed array; the order in which the BigPipe placeholders will start - * execution. Placeholders begin execution in DOM order, except for the - * messages placeholder which must always be executed last. Note that due to + * execution. Placeholders begin execution in DOM order. Note that due to * the Fibers implementation of BigPipe, although placeholders will start * executing in DOM order, they may finish and render in any order. Values * are the BigPipe placeholder IDs. Note that only unique placeholders are @@ -751,35 +736,7 @@ protected function getPlaceholderOrder($html, $placeholders) { foreach ($xpath->query('//span[@data-big-pipe-placeholder-id]') as $node) { $placeholder_ids[] = Html::escape($node->getAttribute('data-big-pipe-placeholder-id')); } - $placeholder_ids = array_unique($placeholder_ids); - - // 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\Core\Messenger\MessengerInterface::addMessage() - // 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 - $message_placeholder_ids = []; - foreach ($placeholders as $placeholder_id => $placeholder_element) { - if (isset($placeholder_element['#lazy_builder']) && $placeholder_element['#lazy_builder'][0] === 'Drupal\Core\Render\Element\StatusMessages::renderMessages') { - $message_placeholder_ids[] = $placeholder_id; - } - } - - // Return placeholder IDs in DOM order, but with the 'status messages' - // placeholders at the end, if they are present. - $ordered_placeholder_ids = array_merge( - array_diff($placeholder_ids, $message_placeholder_ids), - array_intersect($placeholder_ids, $message_placeholder_ids) - ); - return $ordered_placeholder_ids; + return array_unique($placeholder_ids); } /** diff --git a/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php b/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php index 86a4ec44753e0bd365f96d99f33a36f24fe332bf..6894c88a7cc58580b35c71ccffb8a7abd3b438d5 100644 --- a/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php +++ b/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php @@ -175,14 +175,12 @@ public function testBigPipe() { $cases['exception__lazy_builder']->bigPipePlaceholderId => NULL, $cases['exception__embedded_response']->bigPipePlaceholderId => NULL, ], [ - 0 => $cases['edge_case__html_non_lazy_builder']->bigPipePlaceholderId, + 0 => $cases['html']->bigPipePlaceholderId, + 1 => $cases['edge_case__html_non_lazy_builder']->bigPipePlaceholderId, // The suspended placeholder is replaced after the non-suspended // placeholder even though it appears first in the page. // @see Drupal\big_pipe\Render\BigPipe\Render::sendPlaceholders() - 1 => $cases['edge_case__html_non_lazy_builder_suspend']->bigPipePlaceholderId, - // The 'html' case contains the 'status messages' placeholder, which is - // always rendered last. - 2 => $cases['html']->bigPipePlaceholderId, + 2 => $cases['edge_case__html_non_lazy_builder_suspend']->bigPipePlaceholderId, ]); $this->assertSession()->responseContains('</body>'); diff --git a/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php b/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php index a20fe48253127a4a13f308850207905a13beef72..22188d9fc8068eed17722c17fb6a0db9136bd8be 100644 --- a/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php +++ b/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php @@ -82,8 +82,7 @@ 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 = '<div role="contentinfo" aria-label="Status message"'; - + $messages_markup = '<div class="messages messages--status" role="status"'; $test_routes = [ // Messages placeholder rendered first. 'render_placeholder_message_test.first', @@ -104,9 +103,9 @@ public function testMessages_2712935() { $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'); + $assert->elementExists('css', 'div[aria-label="Status message"]'); + $assert->responseContains('aria-label="Status message">P1'); + $assert->responseContains('aria-label="Status message">P2'); // Verify that we end with all messages printed, hence again zero queued. $this->drupalGet(Url::fromRoute('render_placeholder_message_test.queued')); diff --git a/core/modules/big_pipe/tests/src/Unit/Render/FiberPlaceholderTest.php b/core/modules/big_pipe/tests/src/Unit/Render/FiberPlaceholderTest.php index c3c7be0a9e34fa710799f2c198d2c6bb0082bb7c..47f7be38cb40bc8e8bc93ad25e79e8d7469880ac 100644 --- a/core/modules/big_pipe/tests/src/Unit/Render/FiberPlaceholderTest.php +++ b/core/modules/big_pipe/tests/src/Unit/Render/FiberPlaceholderTest.php @@ -7,6 +7,7 @@ use Drupal\big_pipe\Render\BigPipe; use Drupal\big_pipe\Render\BigPipeResponse; use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\Render\ElementInfoManagerInterface; use Drupal\Core\Render\HtmlResponse; use Drupal\Core\Render\PlaceholderGeneratorInterface; @@ -54,7 +55,7 @@ public function testLongPlaceholderFiberSuspendingLoop() { 'languages:language_interface', 'theme', ], - ] + ], ); $session = $this->prophesize(SessionInterface::class); @@ -67,6 +68,7 @@ public function testLongPlaceholderFiberSuspendingLoop() { $this->prophesize(HttpKernelInterface::class)->reveal(), $this->createMock(EventDispatcherInterface::class), $this->prophesize(ConfigFactoryInterface::class)->reveal(), + $this->prophesize(MessengerInterface::class)->reveal(), ); $response = new BigPipeResponse(new HtmlResponse()); diff --git a/core/modules/big_pipe/tests/src/Unit/Render/ManyPlaceholderTest.php b/core/modules/big_pipe/tests/src/Unit/Render/ManyPlaceholderTest.php index 44c6e26886684c1c007edfe8d3e83583df79d1c2..e6bd1811f7057d25e3440d40980c1b6501e6f6f1 100644 --- a/core/modules/big_pipe/tests/src/Unit/Render/ManyPlaceholderTest.php +++ b/core/modules/big_pipe/tests/src/Unit/Render/ManyPlaceholderTest.php @@ -7,6 +7,7 @@ use Drupal\big_pipe\Render\BigPipe; use Drupal\big_pipe\Render\BigPipeResponse; use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\Render\HtmlResponse; use Drupal\Core\Render\RendererInterface; use Drupal\Tests\UnitTestCase; @@ -34,7 +35,8 @@ public function testManyNoJsPlaceHolders() { $this->prophesize(RequestStack::class)->reveal(), $this->prophesize(HttpKernelInterface::class)->reveal(), $this->prophesize(EventDispatcherInterface::class)->reveal(), - $this->prophesize(ConfigFactoryInterface::class)->reveal() + $this->prophesize(ConfigFactoryInterface::class)->reveal(), + $this->prophesize(MessengerInterface::class)->reveal() ); $response = new BigPipeResponse(new HtmlResponse());