diff --git a/core/lib/Drupal/Core/Theme/Registry.php b/core/lib/Drupal/Core/Theme/Registry.php index 9631774..cc17d6f 100644 --- a/core/lib/Drupal/Core/Theme/Registry.php +++ b/core/lib/Drupal/Core/Theme/Registry.php @@ -49,8 +49,11 @@ class Registry { * from; e.g., 'module' for theme hook 'node' of Node module. * - name: The name of the extension the original theme hook originates * from; e.g., 'node' for theme hook 'node' of Node module. - * - theme path: The path to the extension denoted by 'type' and 'name'. - * @todo Remove 'theme path', it's useless. + * - theme path: The effective path_to_theme() during theme(), available as + * 'directory' variable in templates. + * @todo Remove 'theme path', it's useless... or fix it: For theme + * functions, it should point to the respective theme. For templates, + * it should point to the directory that contains the template. * - includes: (optional) An array of include files to load when the theme * hook is executed by theme(). * - file: (optional) A filename to add to 'includes', either prefixed with @@ -310,43 +313,9 @@ protected function build() { // Allow modules to alter the complete registry. drupal_alter('theme_registry', $registry); - // Map base hook suggestions and clean up unnecessary hooks. - foreach ($registry as $hook => &$info) { - if (empty($info['exists'])) { - // Determine whether this is a hook suggestion for an existing hook. - $base_hook = $hook; - // Iteratively strip everything after the last '__' delimiter, until a - // base hook definition is found. Recursive base hooks of base hooks are - // not supported, so this must be an original implementation that points - // to a theme function or template. - while ($pos = strrpos($base_hook, '__')) { - $base_hook = substr($base_hook, 0, $pos); - if (isset($registry[$base_hook]['exists'])) { - break; - } - } - if ($pos !== FALSE && $base_hook !== $hook) { - // If a base hook is found, use it as base. (Pre)processor functions - // of the hook suggestion are appended, since the hook suggestion is - // more specific, by design. Any other info of this hook overrides the - // base hook. - $info = NestedArray::mergeDeep($registry[$base_hook], $info); - $info['base hook'] = $base_hook; - } - else { - // If no base hook was found, then this is a suggestion for a theme - // hook of another extension that is not enabled. - unset($registry[$hook]); - } - } - } - // Clean up unnecessary data. - foreach ($registry as $hook => &$info) { - if (isset($info['exists'])) { - unset($info['exists']); - } - } - return $registry; + $this->registry = $registry; + $this->postProcess(); + return $this->registry; } /** @@ -380,6 +349,8 @@ protected function processExtension(&$registry, $type, $name, $theme_name, $them foreach ($function($registry, $type, $theme_name, $theme_path) as $hook => $info) { // Ensure this hook key exists; it will be set either way. $registry += array($hook => array( + // @todo It's not really clear what these are pointing to, whether they + // are used anywhere, and how, and whether they are needed at all. 'type' => $type, 'name' => $name, )); @@ -397,13 +368,9 @@ protected function processExtension(&$registry, $type, $name, $theme_name, $them $info['exists'] = TRUE; // The effective path_to_theme() during theme(). - // @todo This variable is so horribly broken... remove it. $info['theme path'] = $theme_path; if (isset($info['template'])) { - // A template implementation always takes precedence over functions. - // A potentially existing function pointer is obsolete. - unset($registry[$hook]['function']); // Default the template path to the 'templates' directory of the // extension, unless overridden. if (!isset($info['path'])) { @@ -456,56 +423,189 @@ protected function processExtension(&$registry, $type, $name, $theme_name, $them if ($type == 'theme_engine' && isset($info['template'])) { unset($registry[$hook]['template_file']); } + + if (isset($info['template'])) { + // If it was a function before, and the registry contains (pre)process + // functions, merge them into this hook and remove them from the existing registry, so that . + if (isset($registry[$hook]['function'])) { + foreach (array('preprocess', 'process') as $phase) { + if (isset($registry[$hook][$phase])) { + $info += array($phase => array()); + $info[$phase] = array_merge($registry[$hook][$phase], $info[$phase]); + $registry[$hook][$phase] = array(); + } + } + } + // A template implementation always takes precedence over functions. + // A potentially existing function pointer is obsolete. + unset($registry[$hook]['function']); + // Adjust the effective path_to_theme() during theme(). + $info['theme path'] = $theme_path; + // Default the template path to the 'templates' directory of the + // extension, unless overridden. + if (!isset($info['path'])) { + $info['path'] = $theme_path . '/templates'; + } + } } - // Inject special preprocess functions in their required order. + // Inject preprocess functions in their required order. + // The override logic here is essential: + // - The first time a 'template' is defined by any extension, default + // template (pre)processor functions need to be injected. + // - Some of the template (pre)processor functions have to run first; + // e.g., template_*(). + // - A special variant of template (pre)processor functions, + // template_preprocess_HOOK(), needs to run second, right after the base + // template_(pre)process() functions. + // - Followed by the global hook_(pre)process() functions that apply to + // all templates, which need to be collated from all modules, all + // engines, and all themes (in this order). + // @todo Speaking of "all themes", base themes are missing in the below. + // - And lastly, any other (pre)process functions that have been declared + // in hook_theme(). + // Furthermore: + // - template_preprocess_HOOK() and hook_(pre)process_HOOK() also need to + // run for theme *functions*, not only templates. All other template/ + // default processors are omitted, unless explicitly declared in + // hook_theme(). (performance). + // - If a "later" extension type in the build process replaces a theme + // function with a theme template by declaring 'template' (e.g., a theme + // wants to use a template instead of a function), then all of the + // default processors need to be injected (in the order described above). + // To achieve the required ordering, the build process leverages + // NestedArray::mergeDeep() and all functions that need to be sorted + // specially are added with a string key, and all others are using integer + // keys. Additionally, upon first injection, the special functions are + // prepended to the stack. Due to this, the build process is able to + // re-merge all defined functions again for each extension. // @see theme() - // Also applies if the default implementation is a function, but a theme - // overrides it to a template. - // @todo Collect functions by type base/template/module/*engine/theme, and - // unset + merge the correct results together in a new post-process step - // after build(). - if (!empty($info['exists'])) { - foreach (array('preprocess', 'process') as $phase) { - // 1) The base template_(pre)process(). + foreach (array('preprocess', 'process') as $phase) { + if (isset($info[$phase]) || isset($registry[$hook]['template']) || isset($info['template'])) { + $info += array($phase => array()); + if (!empty($info[$phase])) { + $info[$phase] = array_combine($info[$phase], $info[$phase]); + } $functions = array(); - // These base functions are provided by core and should be loaded - // already, so a function_exists() should be safe (whereas - // template_process() does not exist as of now). - if (function_exists("template_{$phase}")) { - $functions[] = "template_{$phase}"; + // 1) The base template_(pre)process(). + // Not included for functions. Injected on the first occurrence of a + // 'template' property. + // @see theme() + if (isset($info['template'])) { + if (function_exists("template_{$phase}")) { + $functions["template_{$phase}"] = "template_{$phase}"; + } } // 2) The original hook owners' template_(pre)process_HOOK(), if - // registered in $info. - if (isset($info[$phase]) && in_array("template_{$phase}_{$hook}", $info[$phase])) { - $functions[] = "template_{$phase}_{$hook}"; - // Remove it from $info, as the merge would duplicate it otherwise. + // registered in $info, and remove it, as the merge would duplicate it + // otherwise. + // @todo Consider to change this to a generic 'template_' prefix, + // allowing all extensions to inject pre-theme-hook-processors. + if (in_array("template_{$phase}_{$hook}", $info[$phase])) { + $functions["template_{$phase}_{$hook}"] = "template_{$phase}_{$hook}"; $info[$phase] = array_diff($info[$phase], array("template_{$phase}_{$hook}")); } - // 3) Global hook_(pre)process() of all modules. - $functions = array_merge($functions, $this->getHookImplementations($phase)); - // 4) hook_(pre)process_HOOK() implementations and any other preprocess - // hooks, in the vertical processing order of this build mechanism, as - // defined in $info. These do not require manual processing. - - // Prepend the (pre)process functions, unless empty. - if (!empty($functions)) { - $registry[$hook] += array($phase => array()); - $registry[$hook][$phase] = array_merge($functions, $registry[$hook][$phase]); + // 3) Global hook_(pre)process() of all extensions. Not for functions. + // @see theme() + if (isset($info['template'])) { + // Module hook implementations need to be retrieved regardless of + // the current extension type being processed, since themes might + // declare templates, in which case module extensions have been + // processed already. + $functions += $this->getHookImplementations($phase); } + // 4) And the reverse of the above; when template hooks of modules are + // processed for later extension types, the extension's $info normally + // does not override 'template', but yet, we need to add + // hook_(pre)process() functions. + if ($type != 'module' && (isset($registry[$hook]['template']) || isset($info['template']))) { + $function = $name . '_' . $phase; + // @todo The theme registry build should not rely on loaded code. + // Introduce theme_implements(). + if (function_exists($function)) { + $functions[$function] = $function; + } + } + // 5) Append the declared hook_(pre)process_HOOK() functions. They + // are merged in the vertical processing order of this build mechanism. + $info[$phase] = array_merge($functions, $info[$phase]); } } - // Merge this extension's theme hook definition into the existing. - $registry[$hook] = NestedArray::mergeDeep($registry[$hook], $info); - // Themes and theme engines can force-remove all preprocess functions. + // If so, they need to provide their own. Therefore, unset existing before + // merging. // @see hook_theme() if (!empty($info['no preprocess'])) { unset($registry[$hook]['preprocess']); unset($registry[$hook]['no preprocess']); } + + // Merge this extension's theme hook definition into the existing. + $registry[$hook] = NestedArray::mergeDeep($registry[$hook], $info); + } + } + + protected function postProcess() { + // Merge base hooks into suggestions and remove unnecessary hooks. + foreach ($this->registry as $hook => &$info) { + if (empty($info['exists'])) { + if (!$base_hook = $this->getBaseHook($hook)) { + // If no base hook was found, then this is a suggestion for a theme + // hook of another extension that is not enabled. + unset($this->registry[$hook]); + continue; + } + // If a base hook is found, use it as base. (Pre)processor functions + // of the hook suggestion are appended, since the hook suggestion is + // more specific, by design. Any other info of this hook overrides the + // base hook. + $info = NestedArray::mergeDeep($this->registry[$base_hook], $info); + $info['base hook'] = $base_hook; + } + } + + // Clean up unnecessary data. + foreach ($this->registry as $hook => &$info) { + if (isset($info['exists'])) { + unset($info['exists']); + } + if (isset($info['preprocess'])) { + $info['preprocess'] = array_values($info['preprocess']); + } + if (isset($info['process'])) { + $info['process'] = array_values($info['process']); + } + } + + return $this->registry; + } + + /** + * Returns the base hook for a given hook suggestion. + * + * @param string $hook + * The name of a theme hook whose base hook to find. + * + * @return string|false + * The name of the base hook or FALSE. + */ + public function getBaseHook($hook) { + $base_hook = $hook; + // Iteratively strip everything after the last '__' delimiter, until a + // base hook definition is found. Recursive base hooks of base hooks are + // not supported, so the base hook must be an original implementation that + // points to a theme function or template. + while ($pos = strrpos($base_hook, '__')) { + $base_hook = substr($base_hook, 0, $pos); + if (isset($this->registry[$base_hook]['exists'])) { + break; + } + } + if ($pos !== FALSE && $base_hook !== $hook) { + return $base_hook; } + return FALSE; } /** @@ -518,11 +618,12 @@ protected function processExtension(&$registry, $type, $name, $theme_name, $them * An array of module hook implementations; i.e., the actual function names. */ protected function getHookImplementations($hook) { - $implementations = module_implements($hook); - $add_suffix = function ($extension) use ($hook) { - return $extension . '_' . $hook; + $implementations = array(); + foreach (module_implements($hook) as $module) { + $function = $module . '_' . $hook; + $implementations[$function] = $function; }; - return array_map($add_suffix, $implementations); + return $implementations; } /** diff --git a/core/modules/system/lib/Drupal/system/Tests/Theme/RegistryUnitTest.php b/core/modules/system/lib/Drupal/system/Tests/Theme/RegistryUnitTest.php index e736c86..91c7000 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Theme/RegistryUnitTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Theme/RegistryUnitTest.php @@ -56,7 +56,6 @@ function xtestThemeScope() { // included for test_subtheme, even though it is defined. include_once DRUPAL_ROOT . '/' . drupal_get_path('theme', 'test_theme') . '/template.php'; // Retrieve a random hook from its definition. - $registry = $this->getRegistry('test_subtheme')->get(); $this->assertTrue(isset($registry)); @@ -64,9 +63,27 @@ function xtestThemeScope() { } /** + * Tests registration of new hooks in a module. + */ + function xtestModuleNewHooks() { + $registry = $this->getRegistry('test_theme')->get(); + $path = drupal_get_path('module', 'theme_test'); + $file = $path . '/test_theme.inc'; + + // Verify that a new theme function is registered correctly. + $this->assertEqual($registry['test_theme_new_function'], array( + 'type' => 'module', + 'name' => 'theme_test', + 'theme path' => $path, + 'function' => 'theme_test_theme_new_function', + 'variables' => array(), + )); + } + + /** * Tests registration of new hooks in a theme. */ - function testNewThemeHooks() { + function testThemeNewHooks() { $registry = $this->getRegistry('test_theme')->get(); $path = drupal_get_path('theme', 'test_theme'); $file = $path . '/test_theme.inc'; @@ -76,15 +93,8 @@ function testNewThemeHooks() { 'type' => 'theme', 'name' => 'test_theme', 'theme path' => $path, + 'function' => 'test_theme_test_theme_new_function', 'variables' => array(), - 'preprocess' => array( - 'template_preprocess', - 'theme_test_preprocess', - 'test_theme_preprocess', - ), - 'process' => array( - 'theme_test_process', - ), )); // Verify theme functions with includes files. @@ -103,14 +113,17 @@ function testNewThemeHooks() { )); $this->assertNotIsset($registry['test_theme_new_function_preprocess'], 'process'); + // Verify custom theme function, explicitly defined in hook_theme(). + $this->assertEqual($registry['test_theme_new_function_custom']['function'], 'test_theme_new_function_customized'); + // Verify that a new theme template is registered correctly. $this->assertEqual($registry['test_theme_new_template'], array( 'type' => 'theme', 'name' => 'test_theme', + 'variables' => array(), + 'template' => 'test_theme_new_template', 'theme path' => $path, 'path' => $path . '/templates', - 'template' => 'test_theme_new_template', - 'variables' => array(), 'preprocess' => array( 'template_preprocess', 'theme_test_preprocess', @@ -118,18 +131,24 @@ function testNewThemeHooks() { ), 'process' => array( 'theme_test_process', + 'test_theme_process', ), )); // Verify theme templates with additional preprocess functions. + // - template_* preprocessors have to be sorted first. + // - Any additional should be sorted according to the extension type (i.e., + // in this case last, since the theme declared it). $this->assertEqual($registry['test_theme_new_template_preprocess_include']['preprocess'], array( 'template_preprocess', + 'template_preprocess_test_theme_new_template_preprocess_include', 'theme_test_preprocess', 'test_theme_preprocess', - 'template_preprocess_test_theme_new_template_preprocess_include', + 'test_theme_preprocess_test_theme_new_template_preprocess_include', )); $this->assertEqual($registry['test_theme_new_template_preprocess_include']['process'], array( 'theme_test_process', + 'test_theme_process', )); } @@ -138,12 +157,78 @@ function testNewThemeHooks() { */ function testThemeHookExtensions() { $registry = $this->getRegistry('test_theme')->get(); + $path = drupal_get_path('theme', 'test_theme'); // Verify that the non-existing theme hook is not contained. $this->assertNotIsset($registry, 'non_existing_theme_hook'); - // Verify the additional preprocess function for theme('page'). - $this->assertInArray($registry['page']['preprocess'], 'test_theme_preprocess_page'); + // Verify the additional preprocess function for a template. + $this->assertInArray($registry['link']['preprocess'], 'test_theme_preprocess_link'); + $this->assertEqual($registry['link']['preprocess'], array( + 'test_theme_preprocess_link', + )); + $this->assertNotIsset($registry['link'], 'process'); + + // Verify the additional preprocess function for a template. + // Since this is the only and final theme, that preprocess should be last. + $this->assertInArray($registry['html']['preprocess'], 'test_theme_preprocess_html'); + $this->assertEqual($registry['html']['preprocess'], array( + // template_preprocess() + 'template_preprocess', + // template_preprocess_HOOK(), if any. (declarative) + 'template_preprocess_html', + // hook_preprocess(). + 'theme_test_preprocess', + // hook_preprocess_HOOK(), if any. (declarative) + 'theme_test_preprocess_html', + // THEME_preprocess(). + 'test_theme_preprocess', + // THEME_preprocess_HOOK(), if any. (declarative) + 'test_theme_preprocess_html', + )); + $this->assertEqual($registry['html']['process'], array( + // template_process_HOOK(), if any. (declarative) + 'template_process_html', + // hook_process(). + 'theme_test_process', + // THEME_process(). + 'test_theme_process', + )); + + // Verify that a theme is able to replace a function with a template. + $this->assertNotIsset($registry['theme_test_function_replace_template'], 'function'); + $this->assertEqual($registry['theme_test_function_replace_template']['theme path'], $path); + $this->assertEqual($registry['theme_test_function_replace_template']['path'], $path . '/templates'); + $this->assertEqual($registry['theme_test_function_replace_template']['template'], 'theme_test_function_replace_template'); + $this->assertEqual($registry['theme_test_function_replace_template']['preprocess'], array( + 'template_preprocess', + 'template_preprocess_theme_test_function_replace_template', + 'theme_test_preprocess', + 'theme_test_preprocess_theme_test_function_replace_template', + 'test_theme_preprocess', + 'test_theme_preprocess_theme_test_function_replace_template', + )); + $this->assertEqual($registry['theme_test_function_replace_template']['process'], array( + 'theme_test_process', + 'test_theme_process', + )); + } + + /** + * Tests that all registered theme hooks contain a 'theme path'. + * + * Separated from all other tests, since the 'theme path' is architecturally + * broken right now. + */ + function testHookThemePath() { + // drupal_find_theme_functions() and drupal_find_theme_templates() expect + // all theme hooks to have a 'theme path'. + $registry = $this->getRegistry('test_theme')->get(); + foreach ($registry as $hook => $info) { + $this->assertIsset($info, 'theme path', '@value key found for hook @hook.', array( + '@hook' => $hook, + )); + } } /** 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 77019c1..0a205a4 100644 --- a/core/modules/system/tests/modules/theme_test/theme_test.module +++ b/core/modules/system/tests/modules/theme_test/theme_test.module @@ -4,9 +4,20 @@ * Implements hook_theme(). */ function theme_test_theme($existing, $type, $theme, $path) { + // Theme registry tests. + // @see test_theme_theme() + $items['theme_test_function_replace_template'] = array( + 'variables' => array(), + 'preprocess' => array( + 'template_preprocess_theme_test_function_replace_template', + 'theme_test_preprocess_theme_test_function_replace_template', + ), + ); $items['html'] = array( 'preprocess' => array('theme_test_preprocess_html'), ); + + $items['theme_test'] = array( 'file' => 'theme_test.inc', 'variables' => array('foo' => ''), diff --git a/core/modules/system/tests/themes/test_theme/template.php b/core/modules/system/tests/themes/test_theme/template.php index 7220115..b700e95 100644 --- a/core/modules/system/tests/themes/test_theme/template.php +++ b/core/modules/system/tests/themes/test_theme/template.php @@ -32,14 +32,20 @@ function _test_theme_theme_registry() { $theme['test_theme_new_function_include'] = $new + $include; // New function with template preprocess function. $theme['test_theme_new_function_preprocess'] = $new + array( - 'preprocess' => array('template_preprocess_test_theme_new_function_preprocess'), + 'preprocess' => array( + 'template_preprocess_test_theme_new_function_preprocess', + ), ); // New function with template preprocess in an include file. $theme['test_theme_new_function_preprocess_include'] = $new + $include + array( - 'preprocess' => array('template_preprocess_test_theme_new_function_preprocess_include'), + 'preprocess' => array( + 'template_preprocess_test_theme_new_function_preprocess_include', + 'test_theme_preprocess_test_theme_new_function_preprocess_include', + ), + ); + $theme['test_theme_new_function_custom'] = $new + array( + 'function' => 'test_theme_new_function_customized', ); - // @todo explicit function name - // @todo replace existing function // Templates. @@ -50,9 +56,17 @@ function _test_theme_theme_registry() { // New template with template preprocess in an include file. $theme['test_theme_new_template_preprocess_include'] = $new + $include + array( 'template' => 'test_theme_new_template_preprocess_include', - 'preprocess' => array('template_preprocess_test_theme_new_template_preprocess_include'), + 'preprocess' => array( + 'template_preprocess_test_theme_new_template_preprocess_include', + 'test_theme_preprocess_test_theme_new_template_preprocess_include', + ), + ); + // Module function replaced with a template. + $theme['theme_test_function_replace_template'] = array( + 'template' => 'theme_test_function_replace_template', + // And while being there, add another preproces, too :) + 'preprocess' => array('test_theme_preprocess_theme_test_function_replace_template'), ); - // @todo replace function with template // Preprocess functions. @@ -60,11 +74,17 @@ function _test_theme_theme_registry() { $theme['non_existing_theme_hook'] = array( 'preprocess' => array('test_theme_preprocess_non_existing_theme_hook'), ); - // Preprocess function for an existing theme hook. + // Preprocess function for an existing function. + // This implementation actually exists, so as to not break testing sites in + // case test_theme is enabled. + $theme['link'] = array( + 'preprocess' => array('test_theme_preprocess_link'), + ); + // Preprocess function for an existing template. // This implementation actually exists, so as to not break testing sites in // case test_theme is enabled. - $theme['page'] = array( - 'preprocess' => array('test_theme_preprocess_page'), + $theme['html'] = array( + 'preprocess' => array('test_theme_preprocess_html'), ); // @todo preprocess + file/include // @todo no preprocess @@ -89,11 +109,28 @@ function test_theme_preprocess(&$variables, $hook) { } /** - * Preprocess function for theme('page'). + * Implements THEME_process(). + * + * @see _test_theme_theme_registry() + * @see theme() + */ +function test_theme_process(&$variables, $hook) { +} + +/** + * Preprocess function for theme('link'). + * + * @see _test_theme_theme_registry() + */ +function test_theme_preprocess_link(&$variables) { +} + +/** + * Preprocess function for theme('html'). * * @see _test_theme_theme_registry() */ -function test_theme_preprocess_page(&$variables) { +function test_theme_preprocess_html(&$variables) { } /**