Skip to content
Snippets Groups Projects
Verified Commit 44c345dd authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3379885 by catch, Wim Leers: Use MessagesCommand in BigPipe to remove...

Issue #3379885 by catch, Wim Leers: Use MessagesCommand in BigPipe to remove special casing of the messages placeholder
parent eed7ea3a
No related branches found
No related tags found
30 merge requests!9470[10.3.x-only-DO-NOT-MERGE]: #3331771 Fix file_get_contents(): Passing null to parameter,!8540Issue #3457061: Bootstrap Modal dialog Not closing after 10.3.0 Update,!8528Issue #3456871 by Tim Bozeman: Support NULL services,!8373Issue #3427374 by danflanagan8, Vighneshh: taxonomy_tid ViewsArgumentDefault...,!7526Expose roles in response,!7352Draft: Resolve #3203489 "Set filename as",!6791Issue #3163299: Ajax exposed filters not working for multiple instances of the same Views block placed on one page,!3878Removed unused condition head title for views,!3818Issue #2140179: $entity->original gets stale between updates,!3742Issue #3328429: Create item list field formatter for displaying ordered and unordered lists,!3731Claro: role=button on status report items,!3668Resolve #3347842 "Deprecate the trusted",!3651Issue #3347736: Create new SDC component for Olivero (header-search),!3531Issue #3336994: StringFormatter always displays links to entity even if the user in context does not have access,!3355Issue #3209129: Scrolling problems when adding a block via layout builder,!3154Fixes #2987987 - CSRF token validation broken on routes with optional parameters.,!3133core/modules/system/css/components/hidden.module.css,!2812Issue #3312049: [Followup] Fix Drupal.Commenting.FunctionComment.MissingReturnType returns for NULL,!2794Issue #3100732: Allow specifying `meta` data on JSON:API objects,!2378Issue #2875033: Optimize joins and table selection in SQL entity query implementation,!2334Issue #3228209: Add hasRole() method to AccountInterface,!2062Issue #3246454: Add weekly granularity to views date sort,!1105Issue #3025039: New non translatable field on translatable content throws error,!1073issue #3191727: Focus states on mobile second level navigation items fixed,!877Issue #2708101: Default value for link text is not saved,!617Issue #3043725: Provide a Entity Handler for user cancelation,!579Issue #2230909: Simple decimals fail to pass validation,!560Move callback classRemove outside of the loop,!555Issue #3202493,!23Issue #2879087: Use comment access handler instead of hardcoding permissions
Pipeline #101267 passed with warnings
......@@ -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
......
......@@ -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);
}
/**
......
......@@ -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>');
......
......@@ -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'));
......
......@@ -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());
......
......@@ -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());
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment