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

7 files changed, 178 insertions, 129 deletions

diff --git a/includes/theme.inc b/includes/theme.inc
index b1b2f0314..7668ab9e4 100644
--- a/includes/theme.inc
+++ b/includes/theme.inc
@@ -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'])) {
diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index 556c234c2..ca5958ace 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -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(
diff --git a/modules/comment/comment.test b/modules/comment/comment.test
index 889538036..6237ea5b3 100644
--- a/modules/comment/comment.test
+++ b/modules/comment/comment.test
@@ -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.'));
   }
 }
 
diff --git a/modules/field/field.module b/modules/field/field.module
index d6e1e004b..9ad7252a1 100644
--- a/modules/field/field.module
+++ b/modules/field/field.module
@@ -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(
-      'field',
-      'field-name-' . $field_name_css,
-      'field-type-' . $field_type_css,
-      'field-label-' . $element['#label_display'],
-    ),
-    'theme_hook_suggestions' => array(
-      'field',
-      'field__' . $element['#field_name'],
-      'field__' . $element['#bundle'],
-      'field__' . $element['#field_name'] . '__' . $element['#bundle'],
-    ),
+  // 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-' . $variables['field_name_css'],
+    'field-type-' . $variables['field_type_css'],
+    'field-label-' . $element['#label_display'],
   );
-  $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();
-  }
+  // 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'],
+  );
 }
 
 /**
- * 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;
+}
diff --git a/modules/field/theme/field.tpl.php b/modules/field/theme/field.tpl.php
index ee4d5142c..4ac2cfb98 100644
--- a/modules/field/theme/field.tpl.php
+++ b/modules/field/theme/field.tpl.php
@@ -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>
diff --git a/modules/rdf/rdf.module b/modules/rdf/rdf.module
index 36d5b5b35..00a99b0d4 100644
--- a/modules/rdf/rdf.module
+++ b/modules/rdf/rdf.module
@@ -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.
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index e68e17120..da9058d23 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -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',