diff --git a/core/lib/Drupal/Core/Render/Element/StatusMessages.php b/core/lib/Drupal/Core/Render/Element/StatusMessages.php index 4eecd9f42f1a793d6317fefdfb7259315115cc89..0192346e2b31de0f4d23daeab5119cba039a7bfe 100644 --- a/core/lib/Drupal/Core/Render/Element/StatusMessages.php +++ b/core/lib/Drupal/Core/Render/Element/StatusMessages.php @@ -3,6 +3,7 @@ namespace Drupal\Core\Render\Element; use Drupal\Core\Render\Attribute\RenderElement; +use Drupal\big_pipe\Render\Placeholder\BigPipeStrategy; /** * Provides a messages element. @@ -52,6 +53,15 @@ public static function generatePlaceholder(array $element) { $build = [ '#lazy_builder' => [static::class . '::renderMessages', [$element['#display']]], '#create_placeholder' => TRUE, + // Prevent this placeholder being handled by big pipe. Messages are + // very quick to render and this allows pages without other placeholders + // to avoid loading big pipe's JavaScript altogether. Note that while the + // big pipe namespaced is reference, PHP happily uses the '::class' magic + // property without needing to load the class, so this works when big_pipe + // module is not installed. + '#placeholder_strategy_denylist' => [ + BigPipeStrategy::class => TRUE, + ], ]; // Directly create a placeholder as we need this to be placeholdered diff --git a/core/lib/Drupal/Core/Render/Placeholder/ChainedPlaceholderStrategy.php b/core/lib/Drupal/Core/Render/Placeholder/ChainedPlaceholderStrategy.php index ba1417e45a791efe45ed10226d24b505003a38df..c22e430ca2f3132951b2a748a2ebc6a8992bfd60 100644 --- a/core/lib/Drupal/Core/Render/Placeholder/ChainedPlaceholderStrategy.php +++ b/core/lib/Drupal/Core/Render/Placeholder/ChainedPlaceholderStrategy.php @@ -4,6 +4,11 @@ /** * Renders placeholders using a chain of placeholder strategies. + * + * Render arrays may specify a denylist of placeholder strategies by using + * $element['#placeholder_strategy_denylist'][ClassName::class] = TRUE at the + * same level as #lazy_builder. When this is set, placeholder strategies + * specified will be skipped. */ class ChainedPlaceholderStrategy implements PlaceholderStrategyInterface { @@ -43,7 +48,14 @@ public function processPlaceholders(array $placeholders) { // placeholders. The order of placeholder strategies is well defined and // this uses a variation of the "chain of responsibility" design pattern. foreach ($this->placeholderStrategies as $strategy) { - $processed_placeholders = $strategy->processPlaceholders($placeholders); + $candidate_placeholders = []; + foreach ($placeholders as $key => $placeholder) { + if (empty($placeholder['#placeholder_strategy_denylist'][$strategy::class])) { + $candidate_placeholders[$key] = $placeholder; + } + } + + $processed_placeholders = $strategy->processPlaceholders($candidate_placeholders); assert(array_intersect_key($processed_placeholders, $placeholders) === $processed_placeholders, 'Processed placeholders must be a subset of all placeholders.'); $placeholders = array_diff_key($placeholders, $processed_placeholders); $new_placeholders += $processed_placeholders; @@ -52,6 +64,7 @@ public function processPlaceholders(array $placeholders) { break; } } + assert(empty($placeholders), 'It was not possible to replace all placeholders in ChainedPlaceholderStrategy::processPlaceholders()'); return $new_placeholders; } diff --git a/core/lib/Drupal/Core/Render/PlaceholderGenerator.php b/core/lib/Drupal/Core/Render/PlaceholderGenerator.php index 2d93e63471327aaf54c260886a236d173adb84b7..63f8fc3cdac29366824f26b245571eec141792b9 100644 --- a/core/lib/Drupal/Core/Render/PlaceholderGenerator.php +++ b/core/lib/Drupal/Core/Render/PlaceholderGenerator.php @@ -85,15 +85,18 @@ public function shouldAutomaticallyPlaceholder(array $element) { * {@inheritdoc} */ public function createPlaceholder(array $element) { - $placeholder_render_array = array_intersect_key($element, [ + $placeholder_render_array = array_intersect_key($element, \array_flip([ // Placeholders are replaced with markup by executing the associated // #lazy_builder callback, which generates a render array, and which the // Renderer will render and replace the placeholder with. - '#lazy_builder' => TRUE, + '#lazy_builder', // The cacheability metadata for the placeholder. The rendered result of // the placeholder may itself be cached, if [#cache][keys] are specified. - '#cache' => TRUE, - ]); + '#cache', + // The placeholder strategy denylist for this placeholder which can be + // used to skip placeholder strategies for specific render arrays. + '#placeholder_strategy_denylist', + ])); if (isset($element['#lazy_builder_preview'])) { $placeholder_render_array['#preview'] = $element['#lazy_builder_preview']; diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index 0c6e276e3bf6a10b0efacf574145b9f7f794f3f9..8fb473aa8df2fa7f27886d5d77aa51a66e173b9f 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -318,6 +318,7 @@ protected function doRender(&$elements, $is_root_call = FALSE) { '#cache' => TRUE, '#lazy_builder' => TRUE, '#lazy_builder_preview' => TRUE, + '#placeholder_strategy_denylist' => TRUE, '#create_placeholder' => TRUE, ]); @@ -338,6 +339,7 @@ protected function doRender(&$elements, $is_root_call = FALSE) { '#cache', '#create_placeholder', '#lazy_builder_preview', + '#placeholder_strategy_denylist', '#preview', // The keys below are not actually supported, but these are added // automatically by the Renderer. Adding them as though they are diff --git a/core/modules/big_pipe/tests/modules/big_pipe_messages_test/big_pipe_messages_test.info.yml b/core/modules/big_pipe/tests/modules/big_pipe_messages_test/big_pipe_messages_test.info.yml new file mode 100644 index 0000000000000000000000000000000000000000..f7522b47887c239464f794ce06bb2568096cb37d --- /dev/null +++ b/core/modules/big_pipe/tests/modules/big_pipe_messages_test/big_pipe_messages_test.info.yml @@ -0,0 +1,5 @@ +name: 'BigPipe messages test' +type: module +description: 'Forces the messages placeholder to go via the big pipe strategy for testing purposes.' +package: Testing +version: VERSION diff --git a/core/modules/big_pipe/tests/modules/big_pipe_messages_test/src/Hook/BigPipeMessagesHooks.php b/core/modules/big_pipe/tests/modules/big_pipe_messages_test/src/Hook/BigPipeMessagesHooks.php new file mode 100644 index 0000000000000000000000000000000000000000..bb8212bb2848f089b4e333fafacd17c3caa9ae02 --- /dev/null +++ b/core/modules/big_pipe/tests/modules/big_pipe_messages_test/src/Hook/BigPipeMessagesHooks.php @@ -0,0 +1,42 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\big_pipe_messages_test\Hook; + +use Drupal\Core\Hook\Attribute\Hook; +use Drupal\Core\Security\Attribute\TrustedCallback; + +/** + * Hook implementations for big_pipe_test. + */ +class BigPipeMessagesHooks { + + /** + * Implements hook_element_info_alter(). + */ + #[Hook('element_info_alter')] + public function elementInfoAlter(array &$info): void { + $info['status_messages']['#pre_render'][] = static::class . '::preRenderMessages'; + } + + /** + * Pre render callback. + * + * Removes #placeholder_strategy from the messages element to force the + * messages placeholder to go via the big pipe strategy for testing purposes. + */ + #[TrustedCallback] + public static function preRenderMessages(array $element): array { + if (isset($element['#attached']['placeholders'])) { + $key = key($element['#attached']['placeholders']); + unset($element['#attached']['placeholders'][$key]['#placeholder_strategy_denylist']); + } + if (isset($element['messages']['#attached']['placeholders'])) { + $key = key($element['messages']['#attached']['placeholders']); + unset($element['messages']['#attached']['placeholders'][$key]['#placeholder_strategy_denylist']); + } + return $element; + } + +} diff --git a/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php b/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php index 26e92fc14982467c2cf6c2e851653de9007e0758..1ca75bb572628dceee5686df7c25d4144033682d 100644 --- a/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php +++ b/core/modules/big_pipe/tests/src/Functional/BigPipeTest.php @@ -32,7 +32,7 @@ class BigPipeTest extends BrowserTestBase { /** * {@inheritdoc} */ - protected static $modules = ['big_pipe', 'big_pipe_test', 'dblog']; + protected static $modules = ['big_pipe', 'big_pipe_messages_test', 'big_pipe_test', 'dblog']; /** * {@inheritdoc} @@ -88,9 +88,10 @@ public function testNoJsDetection(): void { // 2. Session (authenticated). $this->drupalLogin($this->rootUser); + $this->drupalGet(Url::fromRoute('big_pipe_test')); $this->assertSessionCookieExists('1'); $this->assertBigPipeNoJsCookieExists('0'); - $this->assertSession()->responseContains('<noscript><meta http-equiv="Refresh" content="0; URL=' . base_path() . 'big_pipe/no-js?destination=' . UrlHelper::encodePath(base_path() . 'user/1?check_logged_in=1') . '" />' . "\n" . '</noscript>'); + $this->assertSession()->responseContains('<noscript><meta http-equiv="Refresh" content="0; URL=' . base_path() . 'big_pipe/no-js?destination=' . UrlHelper::encodePath(base_path() . 'big_pipe_test') . '" />' . "\n" . '</noscript>'); $this->assertSession()->responseNotContains($no_js_to_js_markup); $this->assertBigPipeNoJsMetaRefreshRedirect(); $this->assertBigPipeNoJsCookieExists('1'); @@ -103,10 +104,10 @@ public function testNoJsDetection(): void { // 3. Session (anonymous). $this->drupalGet(Url::fromRoute('user.login', [], ['query' => ['trigger_session' => 1]])); - $this->drupalGet(Url::fromRoute('user.login')); + $this->drupalGet(Url::fromRoute('big_pipe_test')); $this->assertSessionCookieExists('1'); $this->assertBigPipeNoJsCookieExists('0'); - $this->assertSession()->responseContains('<noscript><meta http-equiv="Refresh" content="0; URL=' . base_path() . 'big_pipe/no-js?destination=' . base_path() . 'user/login" />' . "\n" . '</noscript>'); + $this->assertSession()->responseContains('<noscript><meta http-equiv="Refresh" content="0; URL=' . base_path() . 'big_pipe/no-js?destination=' . base_path() . 'big_pipe_test" />' . "\n" . '</noscript>'); $this->assertSession()->responseNotContains($no_js_to_js_markup); $this->assertBigPipeNoJsMetaRefreshRedirect(); $this->assertBigPipeNoJsCookieExists('1'); @@ -314,14 +315,9 @@ public function testBigPipeMultiOccurrencePlaceholders(): void { // @see performMetaRefresh() $this->drupalGet(Url::fromRoute('big_pipe_test_multi_occurrence')); - // cspell:disable-next-line - $big_pipe_placeholder_id = 'callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&args%5B0%5D&token=_HAdUpwWmet0TOTe2PSiJuMntExoshbm1kh2wQzzzAA'; - $expected_placeholder_replacement = '<script type="application/vnd.drupal-ajax" data-big-pipe-replacement-for-placeholder-with-id="' . $big_pipe_placeholder_id . '">'; $this->assertSession()->pageTextContains('The count is 1.'); $this->assertSession()->responseNotContains('The count is 2.'); $this->assertSession()->responseNotContains('The count is 3.'); - $raw_content = $this->getSession()->getPage()->getContent(); - $this->assertSame(1, substr_count($raw_content, $expected_placeholder_replacement), 'Only one placeholder replacement was found for the duplicate #lazy_builder arrays.'); // By calling performMetaRefresh() here, we simulate JavaScript being // disabled, because as far as the BigPipe module is concerned, it is diff --git a/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php b/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php index 20244a8fa0984a204f16ce3efe299cc0e554c36a..8e4abff5734ff1e26c109c3923bd7735959427aa 100644 --- a/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php +++ b/core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php @@ -22,6 +22,7 @@ class BigPipeRegressionTest extends WebDriverTestBase { */ protected static $modules = [ 'big_pipe', + 'big_pipe_messages_test', 'big_pipe_regression_test', ]; diff --git a/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php b/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php index f1ca3242a2a8263e63317751e0c7f38eaee21d40..a65b4e01046e09facca07d086c7f503ce8475913 100644 --- a/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php +++ b/core/modules/navigation/tests/src/FunctionalJavascript/PerformanceTest.php @@ -91,7 +91,7 @@ public function testLogin(): void { 'CacheTagInvalidationCount' => 0, 'CacheTagLookupQueryCount' => 14, 'ScriptCount' => 3, - 'ScriptBytes' => 215500, + 'ScriptBytes' => 213000, 'StylesheetCount' => 2, 'StylesheetBytes' => 46000, ]; 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 index 1528a08398163ed42e5489b404854f427a8cf83a..6877871e2d29eb3a7dcf7639da74d26539db38b3 100644 --- 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 @@ -112,7 +112,9 @@ protected function build(array $placeholder_order) { $reordered = []; foreach ($placeholder_order as $placeholder) { - $reordered[$placeholder] = $build['#attached']['placeholders'][$placeholder]; + if (isset($build['#attached']['placeholders'][$placeholder])) { + $reordered[$placeholder] = $build['#attached']['placeholders'][$placeholder]; + } } $build['#attached']['placeholders'] = $reordered; diff --git a/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php b/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php index 4b2779ead48a257a297cee29ffc06fc444bfbed6..63e325c51111c4c0081818da27f9026f82fa1eb3 100644 --- a/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php +++ b/core/modules/system/tests/src/Functional/Render/PlaceholderMessageTest.php @@ -17,7 +17,10 @@ class PlaceholderMessageTest extends BrowserTestBase { /** * {@inheritdoc} */ - protected static $modules = ['render_placeholder_message_test']; + protected static $modules = [ + 'render_placeholder_message_test', + 'big_pipe_messages_test', + ]; /** * {@inheritdoc} diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardJavascriptTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardJavascriptTest.php index f46d71442c14555606266ae68033072ed91ee53a..9a7b1fbb93f6f540e3e2ad78614dee8cceda75b7 100644 --- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardJavascriptTest.php +++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardJavascriptTest.php @@ -35,13 +35,21 @@ public function testBigPipe(): void { ->setPublished(); $node->save(); - // Front page: Five placeholders. + // Front page: Four placeholders. $this->drupalGet(''); + $this->assertBigPipePlaceholderReplacementCount(4); + + // Front page with warm render caches: Zero placeholders. + $this->drupalGet(''); + $this->assertBigPipePlaceholderReplacementCount(0); + + // Node page: Five placeholders. + $this->drupalGet($node->toUrl()); $this->assertBigPipePlaceholderReplacementCount(5); - // Node page: Six placeholders: + // Node page second request: One placeholder for the comment form. $this->drupalGet($node->toUrl()); - $this->assertBigPipePlaceholderReplacementCount(6); + $this->assertBigPipePlaceholderReplacementCount(1); } /** @@ -52,10 +60,17 @@ public function testBigPipe(): void { */ protected function assertBigPipePlaceholderReplacementCount($expected_count): void { $web_assert = $this->assertSession(); - $web_assert->waitForElement('css', 'script[data-big-pipe-event="stop"]'); + if ($expected_count > 0) { + $web_assert->waitForElement('css', 'script[data-big-pipe-event="stop"]'); + } $page = $this->getSession()->getPage(); // Settings are removed as soon as they are processed. - $this->assertCount(0, $this->getDrupalSettings()['bigPipePlaceholderIds']); + if ($expected_count === 0) { + $this->assertArrayNotHasKey('bigPipePlaceholderIds', $this->getDrupalSettings()); + } + else { + $this->assertCount(0, $this->getDrupalSettings()['bigPipePlaceholderIds']); + } $this->assertCount($expected_count, $page->findAll('css', 'script[data-big-pipe-replacement-for-placeholder-with-id]')); } diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php index 44bcc4f648fbec114cda11c1c9d1e24b3adf732e..459b72d59ca2fd630a5d46b98a36f31245bd021e 100644 --- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php +++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php @@ -363,6 +363,10 @@ protected function testLogin(): void { $recorded_queries = $performance_data->getQueries(); $this->assertSame($expected_queries, $recorded_queries); $expected = [ + 'ScriptBytes' => 6500, + 'ScriptCount' => 1, + 'StylesheetBytes' => 1429, + 'StylesheetCount' => 1, 'QueryCount' => 17, 'CacheGetCount' => 69, 'CacheSetCount' => 1, @@ -474,7 +478,7 @@ protected function testLoginBlock(): void { $this->assertSame($expected_queries, $recorded_queries); $expected = [ 'QueryCount' => 18, - 'CacheGetCount' => 104, + 'CacheGetCount' => 103, 'CacheSetCount' => 1, 'CacheDeleteCount' => 1, 'CacheTagInvalidationCount' => 0, diff --git a/core/tests/Drupal/Tests/Core/Render/Placeholder/ChainedPlaceholderStrategyTest.php b/core/tests/Drupal/Tests/Core/Render/Placeholder/ChainedPlaceholderStrategyTest.php index f1344b0ce8e2ae7dc2fe3de42d130e23386d9d80..396d0d5db8fba1bab5fb885651c8d11c1ee5a07a 100644 --- a/core/tests/Drupal/Tests/Core/Render/Placeholder/ChainedPlaceholderStrategyTest.php +++ b/core/tests/Drupal/Tests/Core/Render/Placeholder/ChainedPlaceholderStrategyTest.php @@ -40,20 +40,6 @@ public static function providerProcessPlaceholders() { $prophet = new Prophet(); $data = []; - // Empty placeholders. - $data['empty placeholders'] = [[], [], []]; - - // Placeholder removing strategy. - $placeholders = [ - 'remove-me' => ['#markup' => 'I-am-a-llama-that-will-be-removed-sad-face.'], - ]; - - $prophecy = $prophet->prophesize('\Drupal\Core\Render\Placeholder\PlaceholderStrategyInterface'); - $prophecy->processPlaceholders($placeholders)->willReturn([]); - $dev_null_strategy = $prophecy->reveal(); - - $data['placeholder removing strategy'] = [[$dev_null_strategy], $placeholders, []]; - // Fake Single Flush strategy. $placeholders = [ '67890' => ['#markup' => 'special-placeholder'],