From 6cbb5d9e1e0d904e2e6a8213f7b8602a1b5ab618 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Thu, 16 Oct 2014 13:36:06 +0100 Subject: [PATCH] Issue #2350949 by Wim Leers: Add hook_page_attachments(_alter)() and deprecate hook_page_build/alter(). --- core/includes/common.inc | 89 +++++++--- core/includes/theme.inc | 8 +- .../Core/Page/DefaultHtmlFragmentRenderer.php | 8 +- .../lib/Drupal/Core/Utility/LinkGenerator.php | 4 +- core/modules/block/block.install | 19 --- core/modules/block/block.module | 6 +- .../content_translation.module | 4 +- core/modules/contextual/contextual.module | 13 +- .../tests/language_test/language_test.module | 4 +- core/modules/node/node.module | 4 +- core/modules/quickedit/quickedit.module | 4 +- .../src/Tests/Common/PageRenderTest.php | 95 +++++++++++ .../Tests/System/MainContentFallbackTest.php | 8 +- core/modules/system/system.api.php | 153 ++++++++++-------- core/modules/system/system.module | 41 +++-- .../tests/modules/bc_test/bc_test.info.yml | 6 + .../tests/modules/bc_test/bc_test.module | 44 +++++ .../modules/common_test/common_test.module | 38 +++++ .../modules/system_test/system_test.module | 17 +- .../system_test/system_test.routing.yml | 8 - .../modules/theme_test/theme_test.module | 6 +- core/modules/system/theme.api.php | 20 ++- core/modules/taxonomy/taxonomy.module | 4 +- core/modules/toolbar/src/Element/Toolbar.php | 2 +- core/modules/toolbar/toolbar.module | 8 +- core/modules/tour/tour.module | 6 +- core/modules/update/update.module | 4 +- core/modules/user/user.module | 4 +- core/modules/views/views.module | 20 ++- 29 files changed, 446 insertions(+), 201 deletions(-) delete mode 100644 core/modules/block/block.install create mode 100644 core/modules/system/src/Tests/Common/PageRenderTest.php create mode 100644 core/modules/system/tests/modules/bc_test/bc_test.info.yml create mode 100644 core/modules/system/tests/modules/bc_test/bc_test.module diff --git a/core/includes/common.inc b/core/includes/common.inc index dd4037346d..f6562060d1 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -706,14 +706,14 @@ function drupal_http_header_attributes(array $attributes = array()) { * For authenticated users, the "active" class will be calculated on the * client (through JavaScript), only data- attributes are added to links to * prevent breaking the render cache. The JavaScript is added in - * system_page_build(). + * system_page_attachments(). * - Additional $options elements used by the url() function. * * @return string * An HTML string containing a link to the given path. * * @see _url() - * @see system_page_build() + * @see system_page_attachments() * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.0. * Use \Drupal::l($text, $url) where $url is an instance of * \Drupal\Core\Url. To build a \Drupal\Core\Url object for internal paths @@ -917,8 +917,7 @@ function _drupal_add_html_head_link($attributes, $header = FALSE) { * $options['preprocess'] should be only set to TRUE when a file is required for * all typical visitors and most pages of a site. It is critical that all * preprocessed files are added unconditionally on every page, even if the - * files do not happen to be needed on a page. This is normally done by calling - * _drupal_add_css() in a hook_page_build() implementation. + * files do not happen to be needed on a page. * * Non-preprocessed files should only be added to the page when they are * actually needed. @@ -966,8 +965,8 @@ function _drupal_add_html_head_link($attributes, $header = FALSE) { * page of the website for users for whom it is present at all. This * defaults to FALSE. It is set to TRUE for stylesheets added via module and * theme .info.yml files. Modules that add stylesheets within - * hook_page_build() implementations, or from other code that ensures that - * the stylesheet is added to all website pages, should also set this flag + * hook_page_attachments() implementations, or from other code that ensures + * that the stylesheet is added to all website pages, should also set this flag * to TRUE. All stylesheets within the same group that have the 'every_page' * flag set to TRUE and do not have 'preprocess' set to FALSE are aggregated * together into a single aggregate file, and that aggregate file can be @@ -1367,8 +1366,7 @@ function drupal_clean_id_identifier($id) { * $options['preprocess'] should be only set to TRUE when a file is required for * all typical visitors and most pages of a site. It is critical that all * preprocessed files are added unconditionally on every page, even if the - * files are not needed on a page. This is normally done by calling - * _drupal_add_js() in a hook_page_build() implementation. + * files are not needed on a page. * * Non-preprocessed files should only be added to the page when they are * actually needed. @@ -1411,9 +1409,9 @@ function drupal_clean_id_identifier($id) { * page of the website for users for whom it is present at all. This * defaults to FALSE. It is set to TRUE for JavaScript files that are added * via module and theme .info.yml files. Modules that add JavaScript within - * hook_page_build() implementations, or from other code that ensures that - * the JavaScript is added to all website pages, should also set this flag - * to TRUE. All JavaScript files within the same group and that have the + * hook_page_attachments() implementations, or from other code that ensures + * that the JavaScript is added to all website pages, should also set this + * flag to TRUE. All JavaScript files within the same group and that have the * 'every_page' flag set to TRUE and do not have 'preprocess' set to FALSE * are aggregated together into a single aggregate file, and that aggregate * file can be reused across a user's entire site visit, leading to faster @@ -1712,13 +1710,13 @@ function drupal_get_js($scope = 'header', $javascript = NULL, $skip_alter = FALS * * Example: * @code - * function module1_page_build(&$page) { + * function module1_page_attachments(&$page) { * $page['#attached']['js'][] = array( * 'type' => 'setting', * 'data' => array('foo' => array('a', 'b', 'c')), * ); * } - * function module2_page_build(&$page) { + * function module2_page_attachments(&$page) { * $page['#attached']['js'][] = array( * 'type' => 'setting', * 'data' => array('foo' => array('d')), @@ -2443,7 +2441,10 @@ function drupal_pre_render_links($element) { * @return array * The processed render array for the page. * - * @see hook_page_alter() + * @see hook_page_attachments() + * @see hook_page_attachments_alter() + * @see hook_page_top() + * @see hook_page_bottom() * @see element_info() */ function drupal_prepare_page($page) { @@ -2462,15 +2463,59 @@ function drupal_prepare_page($page) { $page = element_info('page'); } - // Modules can add elements to $page as needed in hook_page_build(). - foreach (\Drupal::moduleHandler()->getImplementations('page_build') as $module) { - $function = $module . '_page_build'; - $function($page); + // Modules can add attachments. + $attachments = []; + foreach (\Drupal::moduleHandler()->getImplementations('page_attachments') as $module) { + $function = $module . '_page_attachments'; + $function($attachments); + } + if (array_diff(array_keys($attachments), ['#attached', '#post_render_cache']) !== []) { + throw new \LogicException('Only #attached and #post_render_cache may be set in hook_page_attachments().'); + } + // Modules and themes can alter page attachments. + \Drupal::moduleHandler()->alter('page_attachments', $attachments); + \Drupal::theme()->alter('page_attachments', $attachments); + if (array_diff(array_keys($attachments), ['#attached', '#post_render_cache']) !== []) { + throw new \LogicException('Only #attached and #post_render_cache may be set in hook_page_attachments_alter().'); + } + if (isset($attachments['#attached'])) { + $page['#attached'] = $attachments['#attached']; + } + if (isset($attachments['#post_render_cache'])) { + $page['#post_render_cache'] = $attachments['#post_render_cache']; + } + + // Modules can add renderable arrays to the top and bottom of the page. + $pseudo_page_top = []; + $pseudo_page_bottom = []; + foreach (\Drupal::moduleHandler()->getImplementations('page_top') as $module) { + $function = $module . '_page_top'; + $function($pseudo_page_top); + } + foreach (\Drupal::moduleHandler()->getImplementations('page_bottom') as $module) { + $function = $module . '_page_bottom'; + $function($pseudo_page_bottom); + } + if (!empty($pseudo_page_top)) { + $page['page_top'] = $pseudo_page_top; + } + if (!empty($pseudo_page_bottom)) { + $page['page_bottom'] = $pseudo_page_bottom; + } + + // @todo Clean this up as part of https://www.drupal.org/node/2352155. + if (\Drupal::moduleHandler()->moduleExists('block')) { + _block_page_build($page); + // Find all non-empty page regions, and add a theme wrapper function that + // allows them to be consistently themed. + $regions = system_region_list(\Drupal::theme()->getActiveTheme()->getName()); + foreach (array_keys($regions) as $region) { + if (!empty($page[$region])) { + $page[$region]['#theme_wrappers'][] = 'region'; + $page[$region]['#region'] = $region; + } + } } - // Modules alter the $page as needed. Blocks are populated into regions like - // 'sidebar_first', 'footer', etc. - \Drupal::moduleHandler()->alter('page', $page); - \Drupal::theme()->alter('page', $page); // If no module has taken care of the main content, add it to the page now. // This allows the site to still be usable even if no modules that diff --git a/core/includes/theme.inc b/core/includes/theme.inc index 0a76479851..723ae63040 100644 --- a/core/includes/theme.inc +++ b/core/includes/theme.inc @@ -925,7 +925,7 @@ function template_preprocess_status_messages(&$variables) { * For authenticated users, the "active" class will be calculated on the * client (through JavaScript), only data- attributes are added to list * items and contained links, to prevent breaking the render cache. The - * JavaScript is added in system_page_build(). + * JavaScript is added in system_page_attachments(). * - heading: (optional) A heading to precede the links. May be an * associative array or a string. If it's an array, it can have the * following elements: @@ -953,7 +953,7 @@ function template_preprocess_status_messages(&$variables) { * * @see \Drupal\Core\Utility\LinkGenerator * @see \Drupal\Core\Utility\LinkGenerator::generate() - * @see system_page_build() + * @see system_page_attachments() */ function template_preprocess_links(&$variables) { $links = $variables['links']; @@ -1920,7 +1920,7 @@ function theme_get_suggestions($args, $base, $delimiter = '__') { * An associative array containing: * - content - An array of page content. * - * @see system_page_build() + * @see system_page_attachments() */ function template_preprocess_maintenance_page(&$variables) { // @todo Rename the templates to page--maintenance + page--install. @@ -1935,7 +1935,7 @@ function template_preprocess_maintenance_page(&$variables) { } $attributes['class'] = $classes; - // @see system_page_build() + // @see system_page_attachments() $variables['#attached']['library'][] = 'core/normalize'; $variables['#attached']['library'][] = 'system/maintenance'; } diff --git a/core/lib/Drupal/Core/Page/DefaultHtmlFragmentRenderer.php b/core/lib/Drupal/Core/Page/DefaultHtmlFragmentRenderer.php index 115edd6fbc..8a229864a5 100644 --- a/core/lib/Drupal/Core/Page/DefaultHtmlFragmentRenderer.php +++ b/core/lib/Drupal/Core/Page/DefaultHtmlFragmentRenderer.php @@ -72,8 +72,12 @@ public function render(HtmlFragmentInterface $fragment, $status_code = 200) { // Persist cache tags associated with this page. Also associate the // "rendered" cache tag. This allows us to invalidate the entire render // cache, regardless of the cache bin. - $cache_tags = $page_array['#cache']['tags']; - $cache_tags[] = 'rendered'; + $cache_tags = Cache::mergeTags( + isset($page_array['page_top']) ? $page_array['page_top']['#cache']['tags'] : [], + $page_array['#cache']['tags'], + isset($page_array['page_bottom']) ? $page_array['page_bottom']['#cache']['tags'] : [], + ['rendered'] + ); // Only keep unique cache tags. We need to prevent duplicates here already // rather than only in the cache layer, because they are also used by // reverse proxies (like Varnish), not only by Drupal's page cache. diff --git a/core/lib/Drupal/Core/Utility/LinkGenerator.php b/core/lib/Drupal/Core/Utility/LinkGenerator.php index 8578195784..a6e8326248 100644 --- a/core/lib/Drupal/Core/Utility/LinkGenerator.php +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php @@ -64,9 +64,9 @@ public function generateFromLink(Link $link) { * For authenticated users, the "active" class will be calculated on the * client (through JavaScript), only data- attributes are added to links to * prevent breaking the render cache. The JavaScript is added in - * system_page_build(). + * system_page_attachments(). * - * @see system_page_build() + * @see system_page_attachments() */ public function generate($text, Url $url) { // Performance: avoid Url::toString() needing to retrieve the URL generator diff --git a/core/modules/block/block.install b/core/modules/block/block.install deleted file mode 100644 index a53a12005c..0000000000 --- a/core/modules/block/block.install +++ /dev/null @@ -1,19 +0,0 @@ -getActiveTheme()->getName(); // Fetch a list of regions for the current theme. diff --git a/core/modules/content_translation/content_translation.module b/core/modules/content_translation/content_translation.module index 01f4221e3b..29653bdb87 100644 --- a/core/modules/content_translation/content_translation.module +++ b/core/modules/content_translation/content_translation.module @@ -722,9 +722,9 @@ function content_translation_preprocess_language_content_settings_table(&$variab } /** - * Implements hook_page_alter(). + * Implements hook_page_attachments(). */ -function content_translation_page_alter(&$page) { +function content_translation_page_attachments(&$page) { $route_match = \Drupal::routeMatch(); // If the current route has no parameters, return. diff --git a/core/modules/contextual/contextual.module b/core/modules/contextual/contextual.module index 66550f205d..10093ba2c0 100644 --- a/core/modules/contextual/contextual.module +++ b/core/modules/contextual/contextual.module @@ -44,15 +44,14 @@ function contextual_toolbar() { } /** - * Implements hook_page_build(). + * Implements hook_page_attachments(). * * Adds the drupal.contextual-links library to the page for any user who has the * 'access contextual links' permission. * * @see contextual_preprocess() */ -function contextual_page_build(&$page) { - +function contextual_page_attachments(array &$page) { if (!\Drupal::currentUser()->hasPermission('access contextual links')) { return; } @@ -89,7 +88,7 @@ function contextual_help($route_name, RouteMatchInterface $route_match) { * Implements hook_preprocess(). * * @see contextual_pre_render_placeholder() - * @see contextual_page_build() + * @see contextual_page_attachments() * @see \Drupal\contextual\ContextualController::render() */ function contextual_preprocess(&$variables, $hook, $info) { @@ -111,9 +110,9 @@ function contextual_preprocess(&$variables, $hook, $info) { // Renders a contextual links placeholder unconditionally, thus not breaking // the render cache. Although the empty placeholder is rendered for all - // users, contextual_page_build() only adds the drupal.contextual-links - // library for users with the 'access contextual links' permission, thus - // preventing unnecessary HTTP requests for users without that permission. + // users, contextual_page_attachments() only adds the asset library for + // users with the 'access contextual links' permission, thus preventing + // unnecessary HTTP requests for users without that permission. $variables['title_suffix']['contextual_links'] = array( '#type' => 'contextual_links_placeholder', '#id' => _contextual_links_to_id($element['#contextual_links']), diff --git a/core/modules/language/tests/language_test/language_test.module b/core/modules/language/tests/language_test/language_test.module index aeb630e2a7..0a891982ad 100644 --- a/core/modules/language/tests/language_test/language_test.module +++ b/core/modules/language/tests/language_test/language_test.module @@ -9,9 +9,9 @@ use Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationUI; /** - * Implements hook_page_build(). + * Implements hook_page_top(). */ -function language_test_page_build() { +function language_test_page_top() { if (\Drupal::moduleHandler()->moduleExists('language')) { language_test_store_language_negotiation(); drupal_set_message(t('Language negotiation method: @name', array('@name' => \Drupal::languageManager()->getNegotiatedLanguageMethod()))); diff --git a/core/modules/node/node.module b/core/modules/node/node.module index eacb7afd14..d4e159fa4e 100644 --- a/core/modules/node/node.module +++ b/core/modules/node/node.module @@ -885,9 +885,9 @@ function node_view_multiple($nodes, $view_mode = 'teaser', $langcode = NULL) { } /** - * Implements hook_page_build(). + * Implements hook_page_top(). */ -function node_page_build(&$page) { +function node_page_top(array &$page) { // Add 'Back to content editing' link on preview page. $route_match = \Drupal::routeMatch(); if ($route_match->getRouteName() == 'entity.node.preview') { diff --git a/core/modules/quickedit/quickedit.module b/core/modules/quickedit/quickedit.module index 6b5a9cf4bc..7ed6f92c46 100644 --- a/core/modules/quickedit/quickedit.module +++ b/core/modules/quickedit/quickedit.module @@ -36,12 +36,12 @@ function quickedit_help($route_name, RouteMatchInterface $route_match) { } /** - * Implements hook_page_build(). + * Implements hook_page_attachments(). * * Adds the quickedit library to the page for any user who has the 'access * in-place editing' permission. */ -function quickedit_page_build(&$page) { +function quickedit_page_attachments(array &$page) { if (!\Drupal::currentUser()->hasPermission('access in-place editing')) { return; } diff --git a/core/modules/system/src/Tests/Common/PageRenderTest.php b/core/modules/system/src/Tests/Common/PageRenderTest.php new file mode 100644 index 0000000000..09dd9b35f2 --- /dev/null +++ b/core/modules/system/src/Tests/Common/PageRenderTest.php @@ -0,0 +1,95 @@ +enableModules(['common_test']); + $this->assertPageRenderHookExceptions('common_test', 'hook_page_attachments'); + } + + /** + * Tests hook_page_attachments_alter() exceptions. + */ + function testHookPageAlter() { + $this->enableModules(['common_test']); + $this->assertPageRenderHookExceptions('common_test', 'hook_page_attachments_alter'); + } + + /** + * Tests hook_page_build() exceptions, a deprecated hook kept around for BC. + */ + function testHookPageBuildExceptions() { + // Also enable the system module, because that module invokes the BC hooks. + $this->enableModules(['bc_test', 'system']); + $this->assertPageRenderHookExceptions('bc_test', 'hook_page_build'); + } + + /** + * Tests hook_page_alter(), a deprecated hook kept around for BC. + */ + function testHookPageAttachmentsAlter() { + // Also enable the system module, because that module invokes the BC hooks. + $this->enableModules(['bc_test', 'system']); + $this->assertPageRenderHookExceptions('bc_test', 'hook_page_alter'); + } + + /** + * Asserts whether expected exceptions are thrown for invalid hook implementations. + * + * @param string $module + * The module whose invalid logic in its hooks to enable. + * @param string $hook + * The page render hook to assert expected exceptions for. + */ + function assertPageRenderHookExceptions($module, $hook) { + // Assert a valid hook implementation doesn't trigger an exception. + $page = []; + drupal_prepare_page($page); + + // Assert an invalid hook implementation doesn't trigger an exception. + \Drupal::state()->set($module . '.' . $hook . '.descendant_attached', TRUE); + $assertion = $hook . '() implementation that sets #attached on a descendant triggers an exception'; + $page = []; + try { + drupal_prepare_page($page); + $this->error($assertion); + } + catch (\LogicException $e) { + $this->pass($assertion); + $this->assertEqual($e->getMessage(), 'Only #attached and #post_render_cache may be set in ' . $hook . '().'); + } + \Drupal::state()->set('bc_test.' . $hook . '.descendant_attached', FALSE); + + // Assert an invalid hook implementation doesn't trigger an exception. + \Drupal::state()->set('bc_test.' . $hook . '.render_array', TRUE); + $assertion = $hook . '() implementation that sets a child render array triggers an exception'; + $page = []; + try { + drupal_prepare_page($page); + $this->error($assertion); + } + catch (\LogicException $e) { + $this->pass($assertion); + $this->assertEqual($e->getMessage(), 'Only #attached and #post_render_cache may be set in ' . $hook . '().'); + } + \Drupal::state()->set($module . '.' . $hook . '.render_array', FALSE); + } + +} diff --git a/core/modules/system/src/Tests/System/MainContentFallbackTest.php b/core/modules/system/src/Tests/System/MainContentFallbackTest.php index 55fb6a9354..288e681208 100644 --- a/core/modules/system/src/Tests/System/MainContentFallbackTest.php +++ b/core/modules/system/src/Tests/System/MainContentFallbackTest.php @@ -61,19 +61,13 @@ function testMainContentFallback() { // Fallback should not trigger when another module is handling content. $this->drupalGet('system-test/main-content-handling'); $this->assertRaw('id="system-test-content"', 'Content handled by another module'); - $this->assertText(t('Content to test main content fallback'), 'Main content still displayed.'); + $this->assertNoText(t('Content to test main content fallback'), 'Main content not displayed.'); // Fallback should trigger when another module // indicates that it is not handling the content. $this->drupalGet('system-test/main-content-fallback'); $this->assertText(t('Content to test main content fallback'), 'Main content fallback properly triggers.'); - // Fallback should not trigger when another module is handling content. - // Note that this test ensures that no duplicate - // content gets created by the fallback. - $this->drupalGet('system-test/main-content-duplication'); - $this->assertNoText(t('Content to test main content fallback'), 'Main content not duplicated.'); - // Request a user* page and see if it is displayed. $this->drupalLogin($this->web_user); $this->drupalGet('user/' . $this->web_user->id() . '/edit'); diff --git a/core/modules/system/system.api.php b/core/modules/system/system.api.php index d9c44f4793..bdbc03f342 100644 --- a/core/modules/system/system.api.php +++ b/core/modules/system/system.api.php @@ -288,26 +288,18 @@ function hook_ajax_render_alter(array &$data) { } /** - * Add elements to a page before it is rendered. + * Add attachments (typically assets) to a page before it is rendered. * - * Use this hook when you want to add elements at the page level. For your - * additions to be printed, they have to be placed below a top level array key - * of the $page array that has the name of a region of the active theme. + * Kept around for backwards compatibility, but now allows only attachments to + * be added, adding renderable arrays is no longer allowed. * - * By default, valid region keys are 'page_top', 'header', 'sidebar_first', - * 'content', 'sidebar_second' and 'page_bottom'. To get a list of all regions - * of the active theme, use system_region_list($theme). Note that $theme is a - * global variable. - * - * If you want to alter the elements added by other modules or if your module - * depends on the elements of other modules, use hook_page_alter() instead which - * runs after this hook. + * @deprecated in Drupal 8.x, will be removed before Drupal 9.0. Successor: + * hook_page_attachments(). Is now effectively an alias of that hook. * * @param $page - * Nested array of renderable elements that make up the page. + * The page to which to add attachments. * - * @see hook_page_alter() - * @see DefaultHtmlFragmentRenderer::render() + * @see hook_page_attachments() */ function hook_page_build(&$page) { $path = drupal_get_path('module', 'foo'); @@ -321,16 +313,76 @@ function hook_page_build(&$page) { if (drupal_is_front_page()) { $page['#attached']['css'][] = $path . '/foo.front.css'; } +} - // Append a standard disclaimer to the content region on a node detail page. - if (\Drupal::request()->attributes->get('node')) { - $page['content']['disclaimer'] = array( - '#markup' => t('Acme, Inc. is not responsible for the contents of this sample code.'), - '#weight' => 25, - ); +/** + * Add attachments (typically assets) to a page before it is rendered. + * + * Use this hook when you want to conditionally add attachments to a page. + * + * If you want to alter the attachments added by other modules or if your module + * depends on the elements of other modules, use hook_page_attachments_alter() + * instead, which runs after this hook. + * + * @param array &$page + * An empty renderable array representing the page. + * + * @see hook_page_attachments_alter() + */ +function hook_page_attachments(array &$page) { + // Unconditionally attach an asset to the page. + $page['#attached']['library'][] = 'core/domready'; + + // Conditionally attach an asset to the page. + if (!\Drupal::currentUser()->hasPermission('may pet kittens')) { + $page['#attached']['library'][] = 'core/jquery'; } } +/** + * Alter attachments (typically assets) to a page before it is rendered. + * + * Use this hook when you want to remove or alter attachments on the page, or + * add attachments to the page that depend on aonther module's attachments (this + * hook runs after hook_page_attachments(). + * + * If you want to alter the attachments added by other modules or if your module + * depends on the elements of other modules, use hook_page_attachments_alter() + * instead, which runs after this hook. + * + * @param array &$page + * An empty renderable array representing the page. + * + * @see hook_page_attachments_alter() + */ +function hook_page_attachments_alter(array &$page) { + // Conditionally remove an asset. + if (in_array('core/jquery', $page['#attached']['library'])) { + $index = array_search('core/jquery', $page['#attached']['library']); + unset($page['#attached']['library'][$index]); + } +} + +/** + * Add a renderable array to the top of the page. + * + * @param array $page_top + * A renderable array representing the top of the page. + */ +function hook_page_top(array &$page_top) { + $page_top['mymodule'] = ['#markup' => 'This is the top.']; +} + +/** + * Add a renderable array to the bottom of the page. + * + * @param array $page_top + * A renderable array representing the bottom of the page. + */ +function hook_page_bottom(array &$page) { + $page_bottom['mymodule'] = ['#markup' => 'This is the bottom.']; +} + /** * Alters all the menu links discovered by the menu link plugin manager. * @@ -546,60 +598,27 @@ function hook_contextual_links_plugins_alter(array &$contextual_links) { /** * Perform alterations before a page is rendered. * - * Use this hook when you want to remove or alter elements at the page - * level, or add elements at the page level that depend on an other module's - * elements (this hook runs after hook_page_build(). - * - * If you are making changes to entities such as forms, menus, or user - * profiles, use those objects' native alter hooks instead (hook_form_alter(), - * for example). - * - * The $page array contains top level elements for each block region: - * @code - * $page['page_top'] - * $page['header'] - * $page['sidebar_first'] - * $page['content'] - * $page['sidebar_second'] - * $page['page_bottom'] - * @endcode - * - * The 'content' element contains the main content of the current page, and its - * structure will vary depending on what module is responsible for building the - * page. Some legacy modules may not return structured content at all: their - * pre-rendered markup will be located in $page['content']['main']['#markup']. + * Kept around for backwards compatibility, but now allows only attachments to + * be added, altering the renderable array for the page is no longer allowed. * - * Pages built by Drupal's core Node module use a standard structure: - * - * @code - * // Node body. - * $page['content']['system_main']['nodes'][$nid]['body'] - * // Array of links attached to the node (add comments, read more). - * $page['content']['system_main']['nodes'][$nid]['links'] - * // The node entity itself. - * $page['content']['system_main']['nodes'][$nid]['#node'] - * // The results pager. - * $page['content']['system_main']['pager'] - * @endcode + * @deprecated in Drupal 8.x, will be removed before Drupal 9.0. Successor: + * hook_page_attachments_alter(). Is now effectively an alias of that hook. * - * Blocks may be referenced by their module/delta pair within a region: - * @code - * // The login block in the first sidebar region. - * $page['sidebar_first']['user_login']['#block']; - * @endcode + * Use this hook when you want to remove or alter attachments at the page + * level, or add attachments at the page level that depend on an other module's + * attachments (this hook runs after hook_page_build(). * * @param $page - * Nested array of renderable elements that make up the page. + * An empty renderable array representing the page. * * @see hook_page_build() - * @see DefaultHtmlFragmentRenderer::render() */ function hook_page_alter(&$page) { - // Add help text to the user login block. - $page['sidebar_first']['user_login']['help'] = array( - '#weight' => -10, - '#markup' => t('To post comments or add content, you first have to log in.'), - ); + // Conditionally remove an asset. + if (in_array('core/jquery', $page['#attached']['library'])) { + $index = array_search('core/jquery', $page['#attached']['library']); + unset($page['#attached']['library'][$index]); + } } /** diff --git a/core/modules/system/system.module b/core/modules/system/system.module index 9376013c36..0d41bbae9b 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -518,12 +518,12 @@ function system_filetransfer_info() { } /** - * Implements hook_page_build(). + * Implements hook_page_attachments(). * * @see template_preprocess_maintenance_page() * @see \Drupal\system\Controller\SystemController::setLinkActiveClass() */ -function system_page_build(&$page) { +function system_page_attachments(array &$page) { // Ensure the same CSS is loaded in template_preprocess_maintenance_page(). $page['#attached']['library'][] = 'core/normalize'; $page['#attached']['library'][] = 'system/base'; @@ -552,6 +552,28 @@ function system_page_build(&$page) { ) ); } + + // Invoke hook_page_build() for modules and hook_page_alter() for both modules + // and themes, for backwards compatibility. + $attachments = []; + foreach (\Drupal::moduleHandler()->getImplementations('page_build') as $module) { + $function = $module . '_page_build'; + $function($attachments); + } + if (array_diff(array_keys($attachments), ['#attached', '#post_render_cache']) !== []) { + throw new \LogicException('Only #attached and #post_render_cache may be set in hook_page_build().'); + } + \Drupal::moduleHandler()->alter('page', $attachments); + \Drupal::theme()->alter('page', $attachments); + if (array_diff(array_keys($attachments), ['#attached', '#post_render_cache']) !== []) { + throw new \LogicException('Only #attached and #post_render_cache may be set in hook_page_alter().'); + } + if (isset($attachments['#attached'])) { + $page['#attached'] = $attachments['#attached']; + } + if (isset($attachments['#post_render_cache'])) { + $page['#post_render_cache'] = $attachments['#post_render_cache']; + } } /** @@ -1226,21 +1248,6 @@ function system_entity_type_build(array &$entity_types) { ->setLinkTemplate('delete-form', 'entity.date_format.delete_form'); } -/** - * Implements hook_page_alter(). - */ -function system_page_alter(&$page) { - // Find all non-empty page regions, and add a theme wrapper function that - // allows them to be consistently themed. - $regions = system_region_list(\Drupal::theme()->getActiveTheme()->getName()); - foreach (array_keys($regions) as $region) { - if (!empty($page[$region])) { - $page[$region]['#theme_wrappers'][] = 'region'; - $page[$region]['#region'] = $region; - } - } -} - /** * Implements hook_block_view_BASE_BLOCK_ID_alter(). */ diff --git a/core/modules/system/tests/modules/bc_test/bc_test.info.yml b/core/modules/system/tests/modules/bc_test/bc_test.info.yml new file mode 100644 index 0000000000..20dbd821db --- /dev/null +++ b/core/modules/system/tests/modules/bc_test/bc_test.info.yml @@ -0,0 +1,6 @@ +name: 'Backwards Compatibility Test' +type: module +description: 'Support module for backwards compatibility tests.' +package: Testing +version: VERSION +core: 8.x diff --git a/core/modules/system/tests/modules/bc_test/bc_test.module b/core/modules/system/tests/modules/bc_test/bc_test.module new file mode 100644 index 0000000000..537acb8379 --- /dev/null +++ b/core/modules/system/tests/modules/bc_test/bc_test.module @@ -0,0 +1,44 @@ +get('bc_test.hook_page_build.descendant_attached', FALSE)) { + $page['content']['#attached']['library'][] = 'core/jquery'; + } + + if (\Drupal::state()->get('bc_test.hook_page_build.render_array', FALSE)) { + $page['something'] = [ + '#markup' => 'test', + ]; + } +} + +/** + * Implements hook_page_alter(). + * + * @see \Drupal\system\Tests\Common\PageRenderTest::assertPageRenderHookExceptions() + */ +function bc_test_page_alter(&$page) { + $page['#attached']['library'][] = 'core/jquery'; + + if (\Drupal::state()->get('bc_test.hook_page_alter.descendant_attached', FALSE)) { + $page['content']['#attached']['library'][] = 'core/jquery'; + } + + if (\Drupal::state()->get('bc_test.hook_page_alter.render_array', FALSE)) { + $page['something'] = [ + '#markup' => 'test', + ]; + } +} diff --git a/core/modules/system/tests/modules/common_test/common_test.module b/core/modules/system/tests/modules/common_test/common_test.module index 3b77d8607e..97ff0bf22b 100644 --- a/core/modules/system/tests/modules/common_test/common_test.module +++ b/core/modules/system/tests/modules/common_test/common_test.module @@ -253,3 +253,41 @@ function common_test_post_render_cache_placeholder(array $element, array $contex return $element; } + +/** + * Implements hook_page_attachments(). + * + * @see \Drupal\system\Tests\Common\PageRenderTest::assertPageRenderHookExceptions() + */ +function common_test_page_attachments(array &$page) { + $page['#attached']['library'][] = 'core/jquery'; + + if (\Drupal::state()->get('common_test.hook_page_attachments.descendant_attached', FALSE)) { + $page['content']['#attached']['library'][] = 'core/jquery'; + } + + if (\Drupal::state()->get('common_test.hook_page_attachments.render_array', FALSE)) { + $page['something'] = [ + '#markup' => 'test', + ]; + } +} + +/** + * Implements hook_page_attachments_alter(). + * + * @see \Drupal\system\Tests\Common\PageRenderTest::assertPageRenderHookExceptions() + */ +function common_test_page_attachments_alter(array &$page) { + $page['#attached']['library'][] = 'core/jquery'; + + if (\Drupal::state()->get('common_test.hook_page_attachments_alter.descendant_attached', FALSE)) { + $page['content']['#attached']['library'][] = 'core/jquery'; + } + + if (\Drupal::state()->get('common_test.hook_page_attachments_alter.render_array', FALSE)) { + $page['something'] = [ + '#markup' => 'test', + ]; + } +} diff --git a/core/modules/system/tests/modules/system_test/system_test.module b/core/modules/system/tests/modules/system_test/system_test.module index 40da5d2e57..78c1504c22 100644 --- a/core/modules/system/tests/modules/system_test/system_test.module +++ b/core/modules/system/tests/modules/system_test/system_test.module @@ -96,22 +96,21 @@ function system_test_lock_exit() { } /** - * Implements hook_page_build(). + * Implements hook_page_attachments(). */ -function system_test_page_build(&$page) { +function system_test_page_attachments(array &$page) { $menu_item['path'] = current_path(); $main_content_display = &drupal_static('system_main_content_added', FALSE); - if ($menu_item['path'] == 'system-test/main-content-handling') { - $page['footer'] = drupal_set_page_content(); - $page['footer']['main']['#markup'] = '
' . $page['footer']['main']['#markup'] . '
'; - } - elseif ($menu_item['path'] == 'system-test/main-content-fallback') { + if ($menu_item['path'] == 'system-test/main-content-fallback') { + // Get the main content, to e.g. dynamically attach an asset. drupal_set_page_content(); + // Indicate we don't want to override the main content. $main_content_display = FALSE; } - elseif ($menu_item['path'] == 'system-test/main-content-duplication') { - drupal_set_page_content(); + elseif ($menu_item['path'] == 'system-test/main-content-handling') { + // Set the main content. + drupal_set_page_content('
Overridden!
'); } // Used by FrontPageTestCase to get the results of drupal_is_front_page(). $frontpage = \Drupal::state()->get('system_test.front_page_output') ?: 0; diff --git a/core/modules/system/tests/modules/system_test/system_test.routing.yml b/core/modules/system/tests/modules/system_test/system_test.routing.yml index 530a2ca28b..9331097ea6 100644 --- a/core/modules/system/tests/modules/system_test/system_test.routing.yml +++ b/core/modules/system/tests/modules/system_test/system_test.routing.yml @@ -29,14 +29,6 @@ system_test.main_content_fallback: requirements: _access: 'TRUE' -system_test.main_content_duplication: - path: '/system-test/main-content-duplication' - defaults: - _title: 'Test main content duplication' - _content: '\Drupal\system_test\Controller\SystemTestController::mainContentFallback' - requirements: - _access: 'TRUE' - system_test.lock_acquire: path: '/system-test/lock-acquire' defaults: diff --git a/core/modules/system/tests/modules/theme_test/theme_test.module b/core/modules/system/tests/modules/theme_test/theme_test.module index e2378fbaa1..25b6ca853b 100644 --- a/core/modules/system/tests/modules/theme_test/theme_test.module +++ b/core/modules/system/tests/modules/theme_test/theme_test.module @@ -69,10 +69,10 @@ function theme_test_preprocess_html(&$variables) { } /** - * Implements hook_page_alter(). + * Implements hook_page_bottom(). */ -function theme_test_page_alter(&$page) { - $page['page_bottom']['theme_test_page_bottom'] = array('#markup' => 'theme test page bottom markup'); +function theme_test_page_bottom(array &$page_bottom) { + $page_bottom['theme_test_page_bottom'] = array('#markup' => 'theme test page bottom markup'); } /** diff --git a/core/modules/system/theme.api.php b/core/modules/system/theme.api.php index 2e41d6a240..87cf94b1d9 100644 --- a/core/modules/system/theme.api.php +++ b/core/modules/system/theme.api.php @@ -160,14 +160,24 @@ * * @section Assets * - * We can distinguish between two types of assets: - * 1. global assets (loaded on all pages where the theme is in use): these are - * defined in the theme's *.info.yml file. - * 2. template-specific assets (loaded on all pages where a specific template is + * We can distinguish between three types of assets: + * 1. unconditional page-level assets (loaded on all pages where the theme is in + * use): these are defined in the theme's *.info.yml file. + * 2. conditional page-level assets (loaded on all pages where the theme is in + * use and a certain condition is met): these are attached in + * hook_page_attachments_alter(), e.g.: + * @code + * function THEME_page_attachments_alter(array &$page) { + * if ($some_condition) { + * $page['#attached']['library'][] = 'mytheme/something'; + * } + * } + * @endcode + * 3. template-specific assets (loaded on all pages where a specific template is * in use): these can be added by in preprocessing functions, using @code * $variables['#attached'] @endcode, e.g.: * @code - * function seven_preprocess_menu_local_action(array &$variables) { + * function THEME_preprocess_menu_local_action(array &$variables) { * // We require Modernizr's touch test for button styling. * $variables['#attached']['library'][] = 'core/modernizr'; * } diff --git a/core/modules/taxonomy/taxonomy.module b/core/modules/taxonomy/taxonomy.module index fc423d6a18..9cc7ba268e 100644 --- a/core/modules/taxonomy/taxonomy.module +++ b/core/modules/taxonomy/taxonomy.module @@ -103,9 +103,9 @@ function taxonomy_term_uri($term) { } /** - * Implements hook_page_build(). + * Implements hook_page_attachments_alter(). */ -function taxonomy_page_build(&$page) { +function taxonomy_page_attachments_alter(array &$page) { $route_match = \Drupal::routeMatch(); if ($route_match->getRouteName() == 'entity.taxonomy_term.canonical' && ($term = $route_match->getParameter('taxonomy_term')) && $term instanceof TermInterface) { foreach ($term->uriRelationships() as $rel) { diff --git a/core/modules/toolbar/src/Element/Toolbar.php b/core/modules/toolbar/src/Element/Toolbar.php index 1b682aaf49..fac39e2adb 100644 --- a/core/modules/toolbar/src/Element/Toolbar.php +++ b/core/modules/toolbar/src/Element/Toolbar.php @@ -64,7 +64,7 @@ public function getInfo() { * @return array * A renderable array. * - * @see toolbar_page_build(). + * @see toolbar_page_top(). */ public static function preRenderToolbar($element) { // Get the configured breakpoints to switch from vertical to horizontal diff --git a/core/modules/toolbar/toolbar.module b/core/modules/toolbar/toolbar.module index a52658929f..9a7914b5eb 100644 --- a/core/modules/toolbar/toolbar.module +++ b/core/modules/toolbar/toolbar.module @@ -47,12 +47,12 @@ function toolbar_theme($existing, $type, $theme, $path) { } /** - * Implements hook_page_build(). + * Implements hook_page_top(). * - * Add admin toolbar to the page_top region automatically. + * Add admin toolbar to the top of the page automatically. */ -function toolbar_page_build(&$page) { - $page['page_top']['toolbar'] = array( +function toolbar_page_top(array &$page_top) { + $page_top['toolbar'] = array( '#type' => 'toolbar', '#access' => \Drupal::currentUser()->hasPermission('access toolbar'), ); diff --git a/core/modules/tour/tour.module b/core/modules/tour/tour.module index dc1e3e97f9..16acf19835 100644 --- a/core/modules/tour/tour.module +++ b/core/modules/tour/tour.module @@ -63,9 +63,9 @@ function tour_toolbar() { } /** - * Implements hook_page_build(). + * Implements hook_page_bottom(). */ -function tour_page_build(&$page) { +function tour_page_bottom(array &$page_bottom) { if (!\Drupal::currentUser()->hasPermission('access tour')) { return; } @@ -85,7 +85,7 @@ function tour_page_build(&$page) { } } if (!empty($tours)) { - $page['help']['tour'] = entity_view_multiple($tours, 'full'); + $page_bottom['tour'] = entity_view_multiple($tours, 'full'); } } } diff --git a/core/modules/update/update.module b/core/modules/update/update.module index 6cb861390a..cdfc40ce8c 100644 --- a/core/modules/update/update.module +++ b/core/modules/update/update.module @@ -114,9 +114,9 @@ function update_help($route_name, RouteMatchInterface $route_match) { } /** - * Implements hook_page_build(). + * Implements hook_page_top(). */ -function update_page_build() { +function update_page_top() { /** @var \Drupal\Core\Routing\AdminContext $admin_context */ $admin_context = \Drupal::service('router.admin_context'); if ($admin_context->isAdminRoute(\Drupal::request()->attributes->get(RouteObjectInterface::ROUTE_OBJECT)) && \Drupal::currentUser()->hasPermission('administer site configuration')) { diff --git a/core/modules/user/user.module b/core/modules/user/user.module index 9146aac13f..c2037de051 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -108,9 +108,9 @@ function user_theme() { } /** - * Implements hook_page_build(). + * Implements hook_page_attachments(). */ -function user_page_build(&$page) { +function user_page_attachments(array &$page) { $path = drupal_get_path('module', 'user'); $page['#attached']['css'][$path . '/css/user.module.css'] = array('every_page' => TRUE); } diff --git a/core/modules/views/views.module b/core/modules/views/views.module index f9b947e441..02e9f52307 100644 --- a/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -281,15 +281,26 @@ function views_theme_suggestions_comment_alter(array &$suggestions, array $varia } /** - * Implements hook_page_alter(). + * Implements hook_element_info_alter(). + * + * @see views_page_display_pre_render() + * @see views_preprocess_page() + */ +function views_element_info_alter(&$types) { + $types['page']['#pre_render'][] = 'views_page_display_pre_render'; +} + +/** + * #pre_render callback to set contextual links for views using a Page display. */ -function views_page_alter(&$page) { +function views_page_display_pre_render(array $element) { // If the main content of this page contains a view, attach its contextual // links to the overall page array. This allows them to be rendered directly // next to the page title. if ($view = views_get_page_view()) { - views_add_contextual_links($page, 'page', $view, $view->current_display); + views_add_contextual_links($element, 'page', $view, $view->current_display); } + return $element; } /** @@ -302,7 +313,8 @@ function views_preprocess_page(&$variables) { } // If the page contains a view as its main content, contextual links may have - // been attached to the page as a whole; for example, by views_page_alter(). + // been attached to the page as a whole; for example, by + // views_page_display_pre_render(). // This allows them to be associated with the page and rendered by default // next to the page title (which we want). However, it also causes the // Contextual Links module to treat the wrapper for the entire page (i.e., -- GitLab