Commit 0329b221 authored by alexpott's avatar alexpott

Issue #2012812 by thedavidmeister, pwieck: drupal_render() can't distinguish...

Issue #2012812 by thedavidmeister, pwieck: drupal_render() can't distinguish between empty strings from theme() from an implemented hook and no hook being matched.
parent 8469e98b
...@@ -4599,28 +4599,40 @@ function drupal_render(&$elements) { ...@@ -4599,28 +4599,40 @@ function drupal_render(&$elements) {
if (!isset($elements['#children'])) { if (!isset($elements['#children'])) {
$elements['#children'] = ''; $elements['#children'] = '';
} }
// Assume that if #theme is set it represents an implemented hook.
$theme_is_implemented = isset($elements['#theme']);
// Call the element's #theme function if it is set. Then any children of the // Call the element's #theme function if it is set. Then any children of the
// element have to be rendered there. If the internal #render_children // element have to be rendered there. If the internal #render_children
// property is set, do not call the #theme function to prevent infinite // property is set, do not call the #theme function to prevent infinite
// recursion. // recursion.
if (isset($elements['#theme']) && !isset($elements['#render_children'])) { if ($theme_is_implemented && !isset($elements['#render_children'])) {
$elements['#children'] = theme($elements['#theme'], $elements); $elements['#children'] = theme($elements['#theme'], $elements);
// If theme() returns FALSE this means that the hook in #theme was not found
// in the registry and so we need to update our flag accordingly. This is
// common for theme suggestions.
$theme_is_implemented = ($elements['#children'] !== FALSE);
} }
// If #theme was not set and the element has children, render them now.
// This is the same process as drupal_render_children() but is inlined // If #theme is not implemented or #render_children is set and the element has
// for speed. // an empty #children attribute, render the children now. This is the same
if ($elements['#children'] === '') { // process as drupal_render_children() but is inlined for speed.
if ((!$theme_is_implemented || isset($elements['#render_children'])) && empty($elements['#children'])) {
foreach ($children as $key) { foreach ($children as $key) {
$elements['#children'] .= drupal_render($elements[$key]); $elements['#children'] .= drupal_render($elements[$key]);
} }
} }
// If #theme was not set, but the element has raw #markup, prepend the content
// in #markup to #children. #children may contain the rendered content // If #theme is not implemented and the element has raw #markup as a
// supplied by #theme, or the rendered child elements, as processed above. If // fallback, prepend the content in #markup to #children. In this case
// both #theme and #markup are set, then #theme is responsible for rendering // #children will contain whatever is provided by #pre_render prepended to
// the element. Eventually assigned #theme_wrappers will expect both the // what is rendered recursively above. If #theme is implemented then it is
// element's #markup and the rendered content of child elements in #children. // the responsibility of that theme implementation to render #markup if
if (!isset($elements['#theme']) && isset($elements['#markup'])) { // required. Eventually #theme_wrappers will expect both #markup and
// #children to be a single string as #children.
if (!$theme_is_implemented && isset($elements['#markup'])) {
$elements['#children'] = $elements['#markup'] . $elements['#children']; $elements['#children'] = $elements['#markup'] . $elements['#children'];
} }
......
...@@ -932,8 +932,9 @@ function drupal_find_base_themes($themes, $key, $used_keys = array()) { ...@@ -932,8 +932,9 @@ function drupal_find_base_themes($themes, $key, $used_keys = array()) {
* properties are mapped to variables expected by the theme hook * properties are mapped to variables expected by the theme hook
* implementations. * implementations.
* *
* @return * @return string|false
* An HTML string representing the themed output. * An HTML string representing the themed output or FALSE if the passed $hook
* is not implemented.
* *
* @see themeable * @see themeable
* @see hook_theme() * @see hook_theme()
...@@ -982,7 +983,10 @@ function theme($hook, $variables = array()) { ...@@ -982,7 +983,10 @@ function theme($hook, $variables = array()) {
if (!isset($candidate)) { if (!isset($candidate)) {
watchdog('theme', 'Theme hook %hook not found.', array('%hook' => $hook), WATCHDOG_WARNING); watchdog('theme', 'Theme hook %hook not found.', array('%hook' => $hook), WATCHDOG_WARNING);
} }
return ''; // There is no theme implementation for the hook passed. Return FALSE so
// the function calling theme() can differentiate between a hook that
// exists and renders an empty string and a hook that is not implemented.
return FALSE;
} }
} }
......
...@@ -72,12 +72,129 @@ function testDrupalRenderBasics() { ...@@ -72,12 +72,129 @@ function testDrupalRenderBasics() {
), ),
'expected' => '', 'expected' => '',
), ),
// Test handling of #markup as a fallback for #theme hooks.
// Simple #markup with no theme.
array( array(
'name' => 'basic renderable array', 'name' => 'basic #markup based renderable array',
'value' => array('#markup' => 'foo'), 'value' => array('#markup' => 'foo'),
'expected' => 'foo', 'expected' => 'foo',
), ),
// Theme suggestion is not implemented, #markup should be rendered.
array(
'name' => '#markup fallback for #theme suggestion not implemented',
'value' => array(
'#theme' => array('suggestionnotimplemented'),
'#markup' => 'foo',
),
'expected' => 'foo',
),
// Theme suggestion is not implemented, child #markup should be rendered.
array(
'name' => '#markup fallback for child elements, #theme suggestion not implemented',
'value' => array(
'#theme' => array('suggestionnotimplemented'),
'child' => array(
'#markup' => 'foo',
),
),
'expected' => 'foo',
),
// Theme suggestion is implemented but returns empty string, #markup
// should not be rendered.
array(
'name' => 'Avoid #markup if #theme is implemented but returns an empty string',
'value' => array(
'#theme' => array('common_test_empty'),
'#markup' => 'foo',
),
'expected' => '',
),
// Theme suggestion is implemented but returns empty string, children
// should not be rendered.
array(
'name' => 'Avoid rendering child elements if #theme is implemented but returns an empty string',
'value' => array(
'#theme' => array('common_test_empty'),
'child' => array(
'#markup' => 'foo',
),
),
'expected' => '',
),
// Test handling of #children and child renderable elements.
// #theme is not set, #children is not set and the array has children.
array(
'name' => '#theme is not set, #children is not set and array has children',
'value' => array(
'child' => array('#markup' => 'bar'),
),
'expected' => 'bar',
),
// #theme is not set, #children is set but empty and the array has
// children.
array(
'name' => '#theme is not set, #children is an empty string and array has children',
'value' => array(
'#children' => '',
'child' => array('#markup' => 'bar'),
),
'expected' => 'bar',
),
// #theme is not set, #children is not empty and will be assumed to be the
// rendered child elements even though the #markup for 'child' differs.
array(
'name' => '#theme is not set, #children is set and array has children',
'value' => array(
'#children' => 'foo',
'child' => array('#markup' => 'bar'),
),
'expected' => 'foo',
),
// #theme is implemented so the values of both #children and 'child' will
// be ignored - it is the responsibility of the theme hook to render these
// if appropriate.
array(
'name' => '#theme is implemented, #children is set and array has children',
'value' => array(
'#theme' => 'common_test_foo',
'#children' => 'baz',
'child' => array('#markup' => 'boo'),
),
'expected' => 'foobar',
),
// #theme is implemented but #render_children is TRUE. As in the case
// where #theme is not set, empty #children means child elements are
// rendered recursively.
array(
'name' => '#theme is implemented, #render_children is TRUE, #children is empty and array has children',
'value' => array(
'#theme' => 'common_test_foo',
'#children' => '',
'#render_children' => TRUE,
'child' => array(
'#markup' => 'boo',
),
),
'expected' => 'boo',
),
// #theme is implemented but #render_children is TRUE. As in the case
// where #theme is not set, #children will take precedence over 'child'.
array(
'name' => '#theme is implemented, #render_children is TRUE, #children is set and array has children',
'value' => array(
'#theme' => 'common_test_foo',
'#children' => 'baz',
'#render_children' => TRUE,
'child' => array(
'#markup' => 'boo',
),
),
'expected' => 'baz',
),
); );
foreach($types as $type) { foreach($types as $type) {
$this->assertIdentical(drupal_render($type['value']), $type['expected'], '"' . $type['name'] . '" input rendered correctly by drupal_render().'); $this->assertIdentical(drupal_render($type['value']), $type['expected'], '"' . $type['name'] . '" input rendered correctly by drupal_render().');
} }
......
...@@ -204,7 +204,7 @@ function testRegistryRebuild() { ...@@ -204,7 +204,7 @@ function testRegistryRebuild() {
// throws an exception. // throws an exception.
$this->rebuildContainer(); $this->rebuildContainer();
$this->container->get('module_handler')->loadAll(); $this->container->get('module_handler')->loadAll();
$this->assertIdentical(theme('theme_test_foo', array('foo' => 'b')), '', 'The theme registry does not contain theme_test_foo, because the module is disabled.'); $this->assertIdentical(theme('theme_test_foo', array('foo' => 'b')), FALSE, 'The theme registry does not contain theme_test_foo, because the module is disabled.');
module_enable(array('theme_test'), FALSE); module_enable(array('theme_test'), FALSE);
// After enabling/disabling a module during a test, we need to rebuild the // After enabling/disabling a module during a test, we need to rebuild the
......
...@@ -164,9 +164,26 @@ function common_test_theme() { ...@@ -164,9 +164,26 @@ function common_test_theme() {
'variables' => array('foo' => 'foo', 'bar' => 'bar'), 'variables' => array('foo' => 'foo', 'bar' => 'bar'),
'template' => 'common-test-foo', 'template' => 'common-test-foo',
), ),
'common_test_empty' => array(
'variables' => array('foo' => 'foo'),
),
); );
} }
/**
* Provides a theme function for drupal_render().
*/
function theme_common_test_foo($variables) {
return $variables['foo'] . $variables['bar'];
}
/**
* Always returns an empty string.
*/
function theme_common_test_empty($variables) {
return '';
}
/** /**
* Implements hook_library_info_alter(). * Implements hook_library_info_alter().
*/ */
......
Markdown is supported
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