Skip to content
GitLab
Commit 414574e1 authored by Alex Bronstein's avatar Alex Bronstein
Browse files

Issue #2559825 by tim.plunkett, Antti J. Salminen, lauriii, aspilicious,... 

Issue #2559825 by tim.plunkett, Antti J. Salminen, lauriii, aspilicious, alexpott: Preprocess functions from base hooks not triggered for theme hooks not using the __  pattern
parent aaf99e1e
Hide whitespace changes
Inline Side-by-side
core/lib/Drupal/Core/Layout/LayoutPluginManager.php
View file @ 414574e1
......@@ -129,10 +129,7 @@ public function processDefinition(&$definition, $plugin_id) {
$template_path .= '/' . implode('/', $template_parts);
}
$definition->setTemplate($template);
// Prepend 'layout__' so the base theme hook will be used.
// @todo Remove this workaround for https://www.drupal.org/node/2559825 in
// https://www.drupal.org/node/2834019.
$definition->setThemeHook('layout__' . strtr($template, '-', '_'));
$definition->setThemeHook(strtr($template, '-', '_'));
$definition->setTemplatePath($template_path);
}
......
core/lib/Drupal/Core/Render/theme.api.php
View file @ 414574e1
......@@ -1138,7 +1138,7 @@ function hook_page_bottom(array &$page_bottom) {
* Instead of this suggestion's implementation being used directly, the base
* hook will be invoked with this implementation as its first suggestion.
* The base hook's files will be included and the base hook's preprocess
* functions will be called in place of any suggestion's preprocess
* functions will be called in addition to any suggestion's preprocess
* functions. If an implementation of hook_theme_suggestions_HOOK() (where
* HOOK is the base hook) changes the suggestion order, a different
* suggestion may be used in place of this suggestion. If after
......
core/lib/Drupal/Core/Theme/Registry.php
View file @ 414574e1
......@@ -606,27 +606,56 @@ protected function processExtension(array &$cache, $name, $type, $theme, $path)
protected function completeSuggestion($hook, array &$cache) {
$previous_hook = $hook;
$incomplete_previous_hook = array();
// Continue looping if the candidate hook doesn't exist or if the candidate
// hook has incomplete preprocess functions, and if the candidate hook is a
// suggestion (has a double underscore).
while ((!isset($cache[$previous_hook]) || isset($cache[$previous_hook]['incomplete preprocess functions']))
&& $pos = strrpos($previous_hook, '__')) {
// Find the first existing candidate hook that has incomplete preprocess
// functions.
if (isset($cache[$previous_hook]) && !$incomplete_previous_hook && isset($cache[$previous_hook]['incomplete preprocess functions'])) {
$incomplete_previous_hook = $cache[$previous_hook];
unset($incomplete_previous_hook['incomplete preprocess functions']);
}
$previous_hook = substr($previous_hook, 0, $pos);
$this->mergePreprocessFunctions($hook, $previous_hook, $incomplete_previous_hook, $cache);
}
// If base hook exists clone of it for the preprocess function
// without a template.
// @see https://www.drupal.org/node/2457295
if (isset($cache[$previous_hook]) && !isset($cache[$previous_hook]['incomplete preprocess functions'])) {
$cache[$hook] = $incomplete_previous_hook + $cache[$previous_hook];
if (isset($incomplete_previous_hook['preprocess functions'])) {
$diff = array_diff($incomplete_previous_hook['preprocess functions'], $cache[$previous_hook]['preprocess functions']);
$cache[$hook]['preprocess functions'] = array_merge($cache[$previous_hook]['preprocess functions'], $diff);
}
// If a base hook isn't set, this is the actual base hook.
if (!isset($cache[$previous_hook]['base hook'])) {
$cache[$hook]['base hook'] = $previous_hook;
}
// In addition to processing suggestions, include base hooks.
if (isset($cache[$hook]['base hook'])) {
// In order to retain the additions from above, pass in the current hook
// as the parent hook, otherwise it will be overwritten.
$this->mergePreprocessFunctions($hook, $cache[$hook]['base hook'], $cache[$hook], $cache);
}
}
/**
* Merges the source hook's preprocess functions into the destination hook's.
*
* @param string $destination_hook_name
* The name of the hook to merge preprocess functions to.
* @param string $source_hook_name
* The name of the hook to merge preprocess functions from.
* @param array $parent_hook
* The parent hook if it exists. Either an incomplete hook from suggestions
* or a base hook.
* @param array $cache
* The theme registry, as documented in
* \Drupal\Core\Theme\Registry::processExtension().
*/
protected function mergePreprocessFunctions($destination_hook_name, $source_hook_name, $parent_hook, array &$cache) {
// If base hook exists clone of it for the preprocess function
// without a template.
// @see https://www.drupal.org/node/2457295
if (isset($cache[$source_hook_name]) && (!isset($cache[$source_hook_name]['incomplete preprocess functions']) || !isset($cache[$destination_hook_name]['incomplete preprocess functions']))) {
$cache[$destination_hook_name] = $parent_hook + $cache[$source_hook_name];
if (isset($parent_hook['preprocess functions'])) {
$diff = array_diff($parent_hook['preprocess functions'], $cache[$source_hook_name]['preprocess functions']);
$cache[$destination_hook_name]['preprocess functions'] = array_merge($cache[$source_hook_name]['preprocess functions'], $diff);
}
// If a base hook isn't set, this is the actual base hook.
if (!isset($cache[$source_hook_name]['base hook'])) {
$cache[$destination_hook_name]['base hook'] = $source_hook_name;
}
}
}
......
core/tests/Drupal/Tests/Core/Layout/LayoutPluginManagerTest.php
View file @ 414574e1
......@@ -119,7 +119,7 @@ public function testGetDefinition() {
$this->assertSame('twocol', $layout_definition->getTemplate());
$this->assertSame("$theme_a_path/templates", $layout_definition->getPath());
$this->assertSame('theme_a/twocol', $layout_definition->getLibrary());
$this->assertSame('layout__twocol', $layout_definition->getThemeHook());
$this->assertSame('twocol', $layout_definition->getThemeHook());
$this->assertSame("$theme_a_path/templates", $layout_definition->getTemplatePath());
$this->assertSame('theme_a', $layout_definition->getProvider());
$this->assertSame('right', $layout_definition->getDefaultRegion());
......@@ -165,7 +165,7 @@ public function testGetDefinition() {
$this->assertSame('plugin-provided-layout', $layout_definition->getTemplate());
$this->assertSame($core_path, $layout_definition->getPath());
$this->assertSame(NULL, $layout_definition->getLibrary());
$this->assertSame('layout__plugin_provided_layout', $layout_definition->getThemeHook());
$this->assertSame('plugin_provided_layout', $layout_definition->getThemeHook());
$this->assertSame("$core_path/templates", $layout_definition->getTemplatePath());
$this->assertSame('core', $layout_definition->getProvider());
$this->assertSame('main', $layout_definition->getDefaultRegion());
......@@ -209,13 +209,13 @@ public function testGetThemeImplementations() {
'layout' => [
'render element' => 'content',
],
'layout__twocol' => [
'twocol' => [
'render element' => 'content',
'base hook' => 'layout',
'template' => 'twocol',
'path' => "$theme_a_path/templates",
],
'layout__plugin_provided_layout' => [
'plugin_provided_layout' => [
'render element' => 'content',
'base hook' => 'layout',
'template' => 'plugin-provided-layout',
......
core/tests/Drupal/Tests/Core/Theme/RegistryTest.php
View file @ 414574e1
......@@ -66,6 +66,13 @@ class RegistryTest extends UnitTestCase {
*/
protected $themeManager;
/**
* The list of functions that get_defined_functions() should provide.
*
* @var array
*/
public static $functions = [];
/**
* {@inheritdoc}
*/
......@@ -82,6 +89,14 @@ protected function setUp() {
$this->setupTheme();
}
/**
* {@inheritdoc}
*/
protected function tearDown() {
parent::tearDown();
static::$functions = [];
}
/**
* Tests getting the theme registry defined by a module.
*/
......@@ -159,6 +174,317 @@ public function testGetRegistryForModule() {
$this->assertTrue(in_array('test_stable_preprocess_theme_test_render_element', $other_registry['theme_test_render_element']['preprocess functions']));
}
/**
* @covers ::postProcessExtension
* @covers ::completeSuggestion
* @covers ::mergePreprocessFunctions
*
* @dataProvider providerTestPostProcessExtension
*
* @param array $defined_functions
* An array of functions to be used in place of get_defined_functions().
* @param array $hooks
* An array of theme hooks to process.
* @param array $expected
* The expected results.
*/
public function testPostProcessExtension($defined_functions, $hooks, $expected) {
static::$functions['user'] = $defined_functions;
$theme = $this->prophesize(ActiveTheme::class);
$theme->getBaseThemes()->willReturn([]);
$theme->getName()->willReturn('test');
$theme->getEngine()->willReturn('twig');
$this->moduleHandler->expects($this->atLeastOnce())
->method('getModuleList')
->willReturn([]);
$class = new \ReflectionClass(TestRegistry::class);
$reflection_method = $class->getMethod('postProcessExtension');
$reflection_method->setAccessible(TRUE);
$reflection_method->invokeArgs($this->registry, [&$hooks, $theme->reveal()]);
$this->assertArrayEquals($expected, $hooks);
}
/**
* Provides test data to ::testPostProcessExtension().
*/
public function providerTestPostProcessExtension() {
// This is test data for unit testing
// \Drupal\Core\Theme\Registry::postProcessExtension(), not what happens
// before it. Therefore, for all test data:
// - Explicitly defined hooks also come with explicitly defined preprocess
// functions, because those are collected in
// \Drupal\Core\Theme\Registry::processExtension().
// - Explicitly defined hooks that set a 'base hook' also have
// 'incomplete preprocess functions' set to TRUE, since that is done in
// \Drupal\Core\Theme\Registry::processExtension().
$data = [];
// Test the discovery of suggestions via the presence of preprocess
// functions that follow the "__" naming pattern.
$data['base_hook_with_autodiscovered_suggestions'] = [
'defined_functions' => [
'test_preprocess_test_hook__suggestion',
'test_preprocess_test_hook__suggestion__another',
],
'hooks' => [
'test_hook' => [
'preprocess functions' => ['explicit_preprocess_test_hook'],
],
],
'expected' => [
'test_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
],
],
'test_hook__suggestion' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'test_preprocess_test_hook__suggestion',
],
'base hook' => 'test_hook',
],
'test_hook__suggestion__another' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'test_preprocess_test_hook__suggestion',
'test_preprocess_test_hook__suggestion__another',
],
'base hook' => 'test_hook',
],
],
];
// Test that suggestions following the "__" naming pattern can also be
// explicitly defined in hook_theme(), such as 'field__node__title' defined
// in node_theme().
$data['base_hook_with_explicit_suggestions'] = [
'defined_functions' => [],
'hooks' => [
'test_hook' => [
'preprocess functions' => ['explicit_preprocess_test_hook'],
],
'test_hook__suggestion__another' => [
'base hook' => 'test_hook',
'preprocess functions' => ['explicit_preprocess_test_hook__suggestion__another'],
'incomplete preprocess functions' => TRUE,
],
],
'expected' => [
'test_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
],
],
'test_hook__suggestion__another' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'explicit_preprocess_test_hook__suggestion__another',
],
'base hook' => 'test_hook',
],
],
];
// Same as above, but also test that a preprocess function for an
// intermediary suggestion level gets discovered.
$data['base_hook_with_explicit_suggestions_and_intermediary_preprocess_function'] = [
'defined_functions' => [
'test_preprocess_test_hook__suggestion',
],
'hooks' => [
'test_hook' => [
'preprocess functions' => ['explicit_preprocess_test_hook'],
],
'test_hook__suggestion__another' => [
'base hook' => 'test_hook',
'preprocess functions' => ['explicit_preprocess_test_hook__suggestion__another'],
'incomplete preprocess functions' => TRUE,
],
],
'expected' => [
'test_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
],
],
'test_hook__suggestion' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'test_preprocess_test_hook__suggestion',
],
'base hook' => 'test_hook',
],
'test_hook__suggestion__another' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'test_preprocess_test_hook__suggestion',
'explicit_preprocess_test_hook__suggestion__another',
],
'base hook' => 'test_hook',
],
],
];
// Test that hooks not following the "__" naming pattern can explicitly
// specify a base hook, such as is done in
// \Drupal\Core\Layout\LayoutPluginManager::getThemeImplementations().
$data['child_hook_without_magic_naming'] = [
'defined_functions' => [],
'hooks' => [
'test_hook' => [
'preprocess functions' => ['explicit_preprocess_test_hook'],
],
'child_hook' => [
'base hook' => 'test_hook',
'preprocess functions' => ['explicit_preprocess_child_hook'],
'incomplete preprocess functions' => TRUE,
],
],
'expected' => [
'test_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
],
],
'child_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'explicit_preprocess_child_hook',
],
'base hook' => 'test_hook',
],
],
];
// Same as above, but also test that such child hooks can also be extended
// with magically named suggestions.
$data['child_hook_with_suggestions'] = [
'defined_functions' => [
'test_preprocess_child_hook__suggestion',
'test_preprocess_child_hook__suggestion__another',
],
'hooks' => [
'test_hook' => [
'preprocess functions' => ['explicit_preprocess_test_hook'],
],
'child_hook' => [
'base hook' => 'test_hook',
'preprocess functions' => ['explicit_preprocess_child_hook'],
'incomplete preprocess functions' => TRUE,
],
],
'expected' => [
'test_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
],
],
'child_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'explicit_preprocess_child_hook',
],
'base hook' => 'test_hook',
],
'child_hook__suggestion' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'explicit_preprocess_child_hook',
'test_preprocess_child_hook__suggestion',
],
'base hook' => 'test_hook',
],
'child_hook__suggestion__another' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'explicit_preprocess_child_hook',
'test_preprocess_child_hook__suggestion',
'test_preprocess_child_hook__suggestion__another',
],
'base hook' => 'test_hook',
],
],
];
// Test that a suggestion following the "__" naming pattern can specify a
// different base hook than what is implied by that pattern. Ensure that
// preprocess functions from both the naming pattern and from 'base hook'
// are collected.
$data['suggestion_with_alternate_base_hook'] = [
'defined_functions' => [
'test_preprocess_test_hook__suggestion',
],
'hooks' => [
'test_hook' => [
'preprocess functions' => ['explicit_preprocess_test_hook'],
],
'alternate_base_hook' => [
'preprocess functions' => ['explicit_preprocess_alternate_base_hook'],
],
'test_hook__suggestion__another' => [
'base hook' => 'alternate_base_hook',
'preprocess functions' => ['explicit_preprocess_test_hook__suggestion__another'],
'incomplete preprocess functions' => TRUE,
],
],
'expected' => [
'test_hook' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
],
],
'alternate_base_hook' => [
'preprocess functions' => [
'explicit_preprocess_alternate_base_hook',
],
],
'test_hook__suggestion' => [
'preprocess functions' => [
'explicit_preprocess_test_hook',
'test_preprocess_test_hook__suggestion',
],
'base hook' => 'test_hook',
],
'test_hook__suggestion__another' => [
'preprocess functions' => [
'explicit_preprocess_alternate_base_hook',
'explicit_preprocess_test_hook',
'test_preprocess_test_hook__suggestion',
'explicit_preprocess_test_hook__suggestion__another',
],
'base hook' => 'alternate_base_hook',
],
],
];
// Test when a base hook is missing.
$data['missing_base_hook'] = [
'defined_functions' => [],
'hooks' => [
'child_hook' => [
'base hook' => 'test_hook',
'preprocess functions' => ['explicit_preprocess_child_hook'],
'incomplete preprocess functions' => TRUE,
],
],
'expected' => [
'child_hook' => [
'preprocess functions' => [
'explicit_preprocess_child_hook',
],
'base hook' => 'test_hook',
],
],
];
return $data;
}
protected function setupTheme() {
$this->registry = new TestRegistry($this->root, $this->cache, $this->lock, $this->moduleHandler, $this->themeHandler, $this->themeInitialization);
$this->registry->setThemeManager($this->themeManager);
......@@ -175,3 +501,14 @@ protected function getPath($module) {
}
}
namespace Drupal\Core\Theme;
use Drupal\Tests\Core\Theme\RegistryTest;
/**
* Overrides get_defined_functions() with a configurable mock.
*/
function get_defined_functions() {
return RegistryTest::$functions ?: \get_defined_functions();
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment