Skip to content

GitLab

Commit 2893abcc authored by Dries's avatar Dries
Browse files
Options

- Patch #652246 by effulgentsia, scor: optimize theme('field') and use it for comment body.

parent 32fc5ac8
Show whitespace changes
Inline Side-by-side
Showing with 178 additions and 129 deletions
includes/theme.inc
View file @ 2893abcc
......@@ -325,10 +325,6 @@ function drupal_theme_rebuild() {
* in hook_theme(). If there is more than one implementation and
* 'render element' is not specified in a later one, then the previous
* definition is kept.
* - 'theme paths': The paths where implementations of a theme hook can be
* found. Its definition is similarly inherited like 'variables'. Each time
* _theme_process_registry() is called for this theme hook, either the
* 'path' key from hook_theme() (if defined) or $path is added.
* - 'preprocess functions': See theme() for detailed documentation.
* - 'process functions': See theme() for detailed documentation.
* @param $name
......@@ -405,14 +401,6 @@ function _theme_process_registry(&$cache, $name, $type, $theme, $path) {
if (!isset($info['path'])) {
$result[$hook]['template'] = $path . '/' . $info['template'];
}
// These are used for template naming suggestions. Theme implementations
// can occur in multiple paths. Suggestions should follow.
if (!isset($info['theme paths']) && isset($cache[$hook])) {
$result[$hook]['theme paths'] = $cache[$hook]['theme paths'];
}
// Check for sub-directories.
$result[$hook]['theme paths'][] = isset($info['path']) ? $info['path'] : $path;
}
// Allow variable processors for all theming hooks, whether the hook is
......@@ -917,6 +905,22 @@ function theme($hook, $variables = array()) {
}
}
// In some cases, a template implementation may not have had
// template_preprocess() run (for example, if the default implementation is
// a function, but a template overrides that default implementation). In
// these cases, a template should still be able to expect to have access to
// the variables provided by template_preprocess(), so we add them here if
// they don't already exist. We don't want to run template_preprocess()
// twice (it would be inefficient and mess up zebra striping), so we use the
// 'directory' variable to determine if it has already run, which while not
// completely intuitive, is reasonably safe, and allows us to save on the
// overhead of adding some new variable to track that.
if (!isset($variables['directory'])) {
$default_template_variables = array();
template_preprocess($default_template_variables, $hook);
$variables += $default_template_variables;
}
// Render the output using the template file.
$template_file = $info['template'] . $extension;
if (isset($info['path'])) {
......
modules/comment/comment.module
View file @ 2893abcc
......@@ -938,33 +938,6 @@ function comment_build_content($comment, $node, $view_mode = 'full') {
entity_prepare_view('comment', array($comment->cid => $comment));
$comment->content += field_attach_view('comment', $comment, $view_mode);
// Prior to Drupal 7, the comment body was a simple text variable, but with
// Drupal 7, it has been upgraded to a field. However, using theme('field') to
// render the comment body results in a noticeable performance degradation for
// pages with many comments. By unsetting #theme, we avoid the overhead of
// theme('field') and instead settle for simply rendering the formatted field
// value that exists as a child element of the 'comment_body' render array,
// which results in equivalent markup and rendering speed as if the comment
// body had not been upgraded to a field. Modules that require the comment
// body to be rendered as a full field (and are willing to accept the
// corresponding performance impact) can restore #theme to 'field' within a
// hook_comment_view() or hook_comment_view_alter() implementation.
// @todo Bypassing theme('field') is not ideal, because:
// - The field label is not displayed, even if its setting is to be
// displayed.
// - hook_preprocess_field() functions do not run, and therefore, attributes
// added in those functions (for example, for RDF) are not output.
// - The HTML markup that's within field.tpl.php is not output, so theme
// developers must use different CSS rules for the comment body than for
// all other fields.
// The goal is for theme('field') to be sufficiently optimized prior to
// Drupal 7 release, so that this code can be removed, and the comment body
// can be rendered just like all other fields. Otherwise, another solution
// to the above problems will be needed. @see http://drupal.org/node/659788.
if (isset($comment->content['comment_body']['#theme']) && ($comment->content['comment_body']['#theme'] === 'field')) {
unset($comment->content['comment_body']['#theme']);
}
if (empty($comment->in_preview)) {
$comment->content['links']['comment'] = array(
'#theme' => 'links__comment',
......@@ -2566,7 +2539,7 @@ function comment_rdf_mapping() {
'type' => 'comment',
'bundle' => RDF_DEFAULT_BUNDLE,
'mapping' => array(
'rdftype' => array('sioc:Post'),
'rdftype' => array('sioc:Post', 'sioct:Comment'),
'title' => array(
'predicates' => array('dc:title'),
),
......@@ -2580,7 +2553,7 @@ function comment_rdf_mapping() {
'datatype' => 'xsd:dateTime',
'callback' => 'date_iso8601',
),
'body' => array(
'comment_body' => array(
'predicates' => array('content:encoded'),
),
'pid' => array(
......
modules/comment/comment.test
View file @ 2893abcc
......@@ -1149,10 +1149,10 @@ class CommentRdfaTestCase extends CommentHelperCase {
// Tests comment #2 as anonymous user.
$this->_testBasicCommentRdfaMarkup($comment2, $anonymous_user);
// Tests the RDFa markup for the homepage (specific to anonymous comments).
$comment_homepage = $this->xpath("//div[@typeof='sioc:Post']//span[@rel='sioc:has_creator']/a[contains(@class, 'username') and @typeof='sioc:User' and @property='foaf:name' and @href='http://example.org/' and contains(@rel, 'foaf:page')]");
$comment_homepage = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//span[@rel='sioc:has_creator']/a[contains(@class, 'username') and @typeof='sioc:User' and @property='foaf:name' and @href='http://example.org/' and contains(@rel, 'foaf:page')]");
$this->assertTrue(!empty($comment_homepage), t('RDFa markup for the homepage of anonymous user found.'));
// There should be no about attribute on anonymous comments.
$comment_homepage = $this->xpath("//div[@typeof='sioc:Post']//span[@rel='sioc:has_creator']/a[@about]");
$comment_homepage = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//span[@rel='sioc:has_creator']/a[@about]");
$this->assertTrue(empty($comment_homepage), t('No about attribute is present on anonymous user comment.'));
// Tests comment #2 as logged in user.
......@@ -1160,10 +1160,10 @@ class CommentRdfaTestCase extends CommentHelperCase {
$this->drupalGet('node/' . $this->node2->nid);
$this->_testBasicCommentRdfaMarkup($comment2, $anonymous_user);
// Tests the RDFa markup for the homepage (specific to anonymous comments).
$comment_homepage = $this->xpath("//div[@typeof='sioc:Post']//span[@rel='sioc:has_creator']/a[contains(@class, 'username') and @typeof='sioc:User' and @property='foaf:name' and @href='http://example.org/' and contains(@rel, 'foaf:page')]");
$comment_homepage = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//span[@rel='sioc:has_creator']/a[contains(@class, 'username') and @typeof='sioc:User' and @property='foaf:name' and @href='http://example.org/' and contains(@rel, 'foaf:page')]");
$this->assertTrue(!empty($comment_homepage), t('RDFa markup for the homepage of anonymous user found.'));
// There should be no about attribute on anonymous comments.
$comment_homepage = $this->xpath("//div[@typeof='sioc:Post']//span[@rel='sioc:has_creator']/a[@about]");
$comment_homepage = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//span[@rel='sioc:has_creator']/a[@about]");
$this->assertTrue(empty($comment_homepage), t('No about attribute is present on anonymous user comment.'));
}
......@@ -1174,20 +1174,22 @@ class CommentRdfaTestCase extends CommentHelperCase {
*
* @param $comment
* Comment object.
* @param $acount
* @param $account
* An array containing information about an anonymous user.
*/
function _testBasicCommentRdfaMarkup($comment, $account = array()) {
$comment_container = $this->xpath("//div[contains(@class, 'comment') and @typeof='sioc:Post']");
$this->assertTrue(!empty($comment_container));
$comment_title = $this->xpath("//div[@typeof='sioc:Post']//h3[@property='dc:title']");
$this->assertEqual((string)$comment_title[0]->a, $comment->subject);
$comment_date = $this->xpath("//div[@typeof='sioc:Post']//*[contains(@property, 'dc:date') and contains(@property, 'dc:created')]");
$this->assertTrue(!empty($comment_date));
$comment_container = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]");
$this->assertTrue(!empty($comment_container), t('Comment RDF type for comment found.'));
$comment_title = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//h3[@property='dc:title']");
$this->assertEqual((string)$comment_title[0]->a, $comment->subject, t('RDFa markup for the comment title found.'));
$comment_date = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//*[contains(@property, 'dc:date') and contains(@property, 'dc:created')]");
$this->assertTrue(!empty($comment_date), t('RDFa markup for the date of the comment found.'));
// The author tag can be either a or span
$comment_author = $this->xpath("//div[@typeof='sioc:Post']//span[@rel='sioc:has_creator']/*[contains(@class, 'username') and @typeof='sioc:User' and @property='foaf:name']");
$comment_author = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//span[@rel='sioc:has_creator']/*[contains(@class, 'username') and @typeof='sioc:User' and @property='foaf:name']");
$name = empty($account['name']) ? $this->web_user->name : $account['name'] . ' (not verified)';
$this->assertEqual((string)$comment_author[0], $name);
$this->assertEqual((string)$comment_author[0], $name, t('RDFa markup for the comment author found.'));
$comment_body = $this->xpath("//div[contains(@class, 'comment') and contains(@typeof, 'sioct:Comment')]//div[@class='content']//div[contains(@class, 'comment-body')]//div[@property='content:encoded']");
$this->assertEqual((string)$comment_body[0]->p, $comment->comment, t('RDFa markup for the comment body found.'));
}
}
......
modules/field/field.module
View file @ 2893abcc
......@@ -177,19 +177,14 @@ function field_help($path, $arg) {
* Implements hook_theme().
*/
function field_theme() {
$path = drupal_get_path('module', 'field') . '/theme';
$items = array(
return array(
'field' => array(
'template' => 'field',
'render element' => 'element',
'path' => $path,
),
'field_multiple_value_form' => array(
'render element' => 'element',
),
);
return $items;
}
/**
......@@ -743,69 +738,145 @@ function field_extract_bundle($obj_type, $bundle) {
}
/**
* Theme preprocess function for field.tpl.php.
* Theme preprocess function for theme_field() and field.tpl.php.
*
* @see theme_field()
* @see field.tpl.php
*/
function template_preprocess_field(&$variables) {
function template_preprocess_field(&$variables, $hook) {
$element = $variables['element'];
// @todo Convert to using drupal_html_class() after benchmarking the impact of
// doing so.
$field_type_css = strtr($element['#field_type'], '_', '-');
$field_name_css = strtr($element['#field_name'], '_', '-');
// Prepare an $items variable that the template can simply loop on.
// Filter out non-children properties that might have been added if the
// renderable array has gone through form_builder().
$items = array_intersect_key($element, array_flip(element_children($element)));
$additions = array(
'object' => $element['#object'],
'view_mode' => $element['#view_mode'],
'items' => $items,
'field_type' => $element['#field_type'],
'field_name' => $element['#field_name'],
'field_type_css' => $field_type_css,
'field_name_css' => $field_name_css,
'label' => check_plain($element['#title']),
'label_display' => $element['#label_display'],
'label_hidden' => $element['#label_display'] == 'hidden',
'field_language' => $element['#language'],
'field_translatable' => $element['#field_translatable'],
'classes_array' => array(
// There's some overhead in calling check_plain() so only call it if the label
// variable is being displayed. Otherwise, set it to NULL to avoid PHP
// warnings if a theme implementation accesses the variable even when it's
// supposed to be hidden. If a theme implementation needs to print a hidden
// label, it needs to supply a preprocess function that sets it to the
// sanitized element title or whatever else is wanted in its place.
$variables['label_hidden'] = ($element['#label_display'] == 'hidden');
$variables['label'] = $variables['label_hidden'] ? NULL : check_plain($element['#title']);
// We want other preprocess functions and the theme implementation to have
// fast access to the field item render arrays. The item render array keys
// (deltas) should always be a subset of the keys in #items, and looping on
// those keys is faster than calling element_children() or looping on all keys
// within $element, since that requires traversal of all element properties.
$variables['items'] = array();
foreach ($element['#items'] as $delta => $item) {
if (!empty($element[$delta])) {
$variables['items'][$delta] = $element[$delta];
}
}
// Add default CSS classes. Since there can be many fields rendered on a page,
// save some overhead by calling strtr() directly instead of
// drupal_html_class().
$variables['field_name_css'] = strtr($element['#field_name'], '_', '-');
$variables['field_type_css'] = strtr($element['#field_type'], '_', '-');
$variables['classes_array'] = array(
'field',
'field-name-' . $field_name_css,
'field-type-' . $field_type_css,
'field-name-' . $variables['field_name_css'],
'field-type-' . $variables['field_type_css'],
'field-label-' . $element['#label_display'],
),
'theme_hook_suggestions' => array(
'field',
);
// Add specific suggestions that can override the default implementation.
$variables['theme_hook_suggestions'] = array(
'field__' . $element['#field_name'],
'field__' . $element['#bundle'],
'field__' . $element['#field_name'] . '__' . $element['#bundle'],
),
);
$variables = array_merge($variables, $additions);
// Initialize attributes for each item.
$variables['item_attributes_array'] = array();
foreach ($variables['items'] as $delta => $item) {
$variables['item_attributes_array'][$delta] = array();
}
}
/**
* Theme process function for field.tpl.php.
* Theme process function for theme_field() and field.tpl.php.
*
* @see theme_field()
* @see field.tpl.php
*/
function template_process_field(&$variables) {
// Flatten out attributes for each item.
function template_process_field(&$variables, $hook) {
// The default theme implementation is a function, so template_process() does
// not automatically run, so we need to flatten the classes and attributes
// here. For best performance, only call drupal_attributes() when needed, and
// note that template_preprocess_field() does not initialize the
// *_attributes_array variables.
$variables['classes'] = implode(' ', $variables['classes_array']);
$variables['attributes'] = empty($variables['attributes_array']) ? '' : drupal_attributes($variables['attributes_array']);
$variables['title_attributes'] = empty($variables['title_attributes_array']) ? '' : drupal_attributes($variables['title_attributes_array']);
$variables['content_attributes'] = empty($variables['content_attributes_array']) ? '' : drupal_attributes($variables['content_attributes_array']);
foreach ($variables['items'] as $delta => $item) {
$variables['item_attributes'][$delta] = drupal_attributes($variables['item_attributes_array'][$delta]);
$variables['item_attributes'][$delta] = empty($variables['item_attributes_array'][$delta]) ? '' : drupal_attributes($variables['item_attributes_array'][$delta]);
}
}
/**
* @} End of "defgroup field"
*/
/**
* Returns a themed field.
*
* This is the default theme implementation to display the value of a field.
* Theme developers who are comfortable with overriding theme functions may do
* so in order to customize this markup. This function can be overridden with
* varying levels of specificity. For example, for a field named 'body'
* displayed on the 'article' content type, any of the following functions will
* override this default implementation. The first of these functions that
* exists is used:
* - THEMENAME_field__body__article()
* - THEMENAME_field__article()
* - THEMENAME_field__body()
* - THEMENAME_field()
*
* Theme developers who prefer to customize templates instead of overriding
* functions may copy the "field.tpl.php" from the "modules/field/theme" folder
* of the Drupal installation to somewhere within the theme's folder and
* customize it, just like customizing other Drupal templates such as
* page.tpl.php or node.tpl.php. However, it takes longer for the server to
* process templates than to call a function, so for websites with many fields
* displayed on a page, this can result in a noticeable slowdown of the website.
* For these websites, developers are discouraged from placing a field.tpl.php
* file into the theme's folder, but may customize templates for specific
* fields. For example, for a field named 'body' displayed on the 'article'
* content type, any of the following templates will override this default
* implementation. The first of these templates that exists is used:
* - field--body--article.tpl.php
* - field--article.tpl.php
* - field--body.tpl.php
* - field.tpl.php
* So, if the body field on the article content type needs customization, a
* field--body--article.tpl.php file can be added within the theme's folder.
* Because it's a template, it will result in slightly more time needed to
* display that field, but it will not impact other fields, and therefore,
* is unlikely to cause a noticeable change in website performance. A very rough
* guideline is that if a page is being displayed with more than 100 fields and
* they are all themed with a template instead of a function, it can add up to
* 5% to the time it takes to display that page. This is a guideline only and
* the exact performance impact depends on the server configuration and the
* details of the website.
*
* @see template_preprocess_field()
* @see template_process_field()
* @see field.tpl.php
*
* @ingroup themeable
*/
function theme_field($variables) {
$output = '';
// Render the label, if it's not hidden.
if (!$variables['label_hidden']) {
$output .= '<div class="field-label"' . $variables['title_attributes'] . '>' . $variables['label'] . ':&nbsp;</div>';
}
// Render the items.
$output .= '<div class="field-items"' . $variables['content_attributes'] . '>';
foreach ($variables['items'] as $delta => $item) {
$classes = 'field-item ' . ($delta % 2 ? 'odd' : 'even');
$output .= '<div class="' . $classes . '"' . $variables['item_attributes'][$delta] . '>' . drupal_render($item) . '</div>';
}
$output .= '</div>';
// Render the top-level DIV.
$output = '<div class="' . $variables['classes'] . ' clearfix"' . $variables['attributes'] . '>' . $output . '</div>';
return $output;
}
modules/field/theme/field.tpl.php
View file @ 2893abcc
......@@ -3,7 +3,10 @@
/**
* @file field.tpl.php
* Default theme implementation to display the value of a field.
* Default template implementation to display the value of a field.
*
* This file is not used and is here as a starting point for customization only.
* @see theme_field().
*
* Available variables:
* - $items: An array of field values. Use render() to output them.
......@@ -19,25 +22,33 @@
* "field-name-field-description".
* - field-type-[field_type]: The current field type. For example, if the
* field type is "text" it would result in "field-type-text".
* - field-label-[label_display]: The current label position. For example, if the
* label position is "above" it would result in "field-label-above".
* - field-label-[label_display]: The current label position. For example, if
* the label position is "above" it would result in "field-label-above".
*
* Other variables:
* - $object: The object to which the field is attached.
* - $view_mode: View mode, e.g. 'full', 'teaser'...
* - $field_name: The field name.
* - $field_type: The field type.
* - $element['#object']: The object to which the field is attached.
* - $element['#view_mode']: View mode, e.g. 'full', 'teaser'...
* - $element['#field_name']: The field name.
* - $element['#field_type']: The field type.
* - $element['#field_language']: The field language.
* - $element['#field_translatable']: Whether the field is translatable or not.
* - $element['#label_display']: Position of label display, inline, above, or
* hidden.
* - $field_name_css: The css-compatible field name.
* - $field_type_css: The css-compatible field type.
* - $field_language: The field language.
* - $field_translatable: Whether the field is translatable or not.
* - $label_display: Position of label display, inline, above, or hidden.
* - $classes_array: Array of html class attribute values. It is flattened
* into a string within the variable $classes.
*
* @see template_preprocess_field()
* @see theme_field()
*/
?>
<!--
THIS FILE IS NOT USED AND IS HERE AS A STARTING POINT FOR CUSTOMIZATION ONLY.
See http://api.drupal.org/api/function/theme_field/7 for details.
After copying this file to your theme's folder and customizing it, remove this
HTML comment.
-->
<div class="<?php print $classes; ?> clearfix"<?php print $attributes; ?>>
<?php if (!$label_hidden) : ?>
<div class="field-label"<?php print $title_attributes; ?>><?php print $label ?>:&nbsp;</div>
......
modules/rdf/rdf.module
View file @ 2893abcc
......@@ -570,13 +570,6 @@ function rdf_preprocess_comment(&$variables) {
$variables['title_attributes_array']['property'] = $comment->rdf_mapping['title']['predicates'];
$variables['title_attributes_array']['datatype'] = '';
}
if (!empty($comment->rdf_mapping['body'])) {
// We need a special case here since the comment body is not a field. Note
// that for that reason, fields attached to comment will be ignored by RDFa
// parsers since we set the property attribute here.
// @todo Use fields instead, see http://drupal.org/node/538164
$variables['content_attributes_array']['property'] = $comment->rdf_mapping['body']['predicates'];
}
// Annotates the parent relationship between the current comment and the node
// it belongs to. If available, the parent comment is also annotated.
......
modules/system/system.api.php
View file @ 2893abcc
......@@ -1084,8 +1084,6 @@ function hook_permission() {
* 'module', 'theme_engine', or 'theme'.
* - theme path: (automatically derived) The directory path of the theme or
* module, so that it doesn't need to be looked up.
* - theme paths: (automatically derived) An array of template suggestions where
* .tpl.php files related to this theme hook may be found.
*
* The following parameters are all optional.
*
......@@ -1162,9 +1160,6 @@ function hook_theme($existing, $type, $theme, $path) {
* 'file' => 'modules/user/user.pages.inc',
* 'type' => 'module',
* 'theme path' => 'modules/user',
* 'theme paths' => array(
* 0 => 'modules/user',
* ),
* 'preprocess functions' => array(
* 0 => 'template_preprocess',
* 1 => 'template_preprocess_user_profile',
......
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