#878092 follow-up by sun, David_Rothstein: Fixed Regression from D7 alpha: themes are unable to render one group of node links separately from another.

9 files changed, 244 insertions, 10 deletions

diff --git a/includes/common.inc b/includes/common.inc
index 3b97014ec..863b7c099 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -5256,6 +5256,97 @@ function drupal_pre_render_link($element) {
 }
 
 /**
+ * #pre_render callback that collects child links into a single array.
+ *
+ * This function can be added as a pre_render callback for a renderable array,
+ * usually one which will be themed by theme_links(). It iterates through all
+ * unrendered children of the element, collects any #links properties it finds,
+ * merges them into the parent element's #links array, and prevents those
+ * children from being rendered separately.
+ *
+ * The purpose of this is to allow links to be logically grouped into related
+ * categories, so that each child group can be rendered as its own list of
+ * links if drupal_render() is called on it, but calling drupal_render() on the
+ * parent element will still produce a single list containing all the remaining
+ * links, regardless of what group they were in.
+ *
+ * A typical example comes from node links, which are stored in a renderable
+ * array similar to this:
+ * @code
+ * $node->content['links'] = array(
+ *   '#theme' => 'links__node',
+ *   '#pre_render' = array('drupal_pre_render_links'),
+ *   'comment' => array(
+ *     '#theme' => 'links__node__comment',
+ *     '#links' => array(
+ *       // An array of links associated with node comments, suitable for
+ *       // passing in to theme_links().
+ *     ),
+ *   ),
+ *   'statistics' => array(
+ *     '#theme' => 'links__node__statistics',
+ *     '#links' => array(
+ *       // An array of links associated with node statistics, suitable for
+ *       // passing in to theme_links().
+ *     ),
+ *   ),
+ *   'translation' => array(
+ *     '#theme' => 'links__node__translation',
+ *     '#links' => array(
+ *       // An array of links associated with node translation, suitable for
+ *       // passing in to theme_links().
+ *     ),
+ *   ),
+ * );
+ * @endcode
+ *
+ * In this example, the links are grouped by functionality, which can be
+ * helpful to themers who want to display certain kinds of links independently.
+ * For example, adding this code to node.tpl.php will result in the comment
+ * links being rendered as a single list:
+ * @code
+ * print render($content['links']['comment']);
+ * @endcode
+ *
+ * (where $node->content has been transformed into $content before handing
+ * control to the node.tpl.php template).
+ *
+ * The pre_render function defined here allows the above flexibility, but also
+ * allows the following code to be used to render all remaining links into a
+ * single list, regardless of their group:
+ * @code
+ * print render($content['links']);
+ * @endcode
+ *
+ * In the above example, this will result in the statistics and translation
+ * links being rendered together in a single list (but not the comment links,
+ * which were rendered previously on their own).
+ *
+ * Because of the way this function works, the individual properties of each
+ * group (for example, a group-specific #theme property such as
+ * 'links__node__comment' in the example above, or any other property such as
+ * #attributes or #pre_render that is attached to it) are only used when that
+ * group is rendered on its own. When the group is rendered together with other
+ * children, these child-specific properties are ignored, and only the overall
+ * properties of the parent are used.
+ */
+function drupal_pre_render_links($element) {
+  $element += array('#links' => array());
+  foreach (element_children($element) as $key) {
+    $child = &$element[$key];
+    // If the child has links which have not been printed yet and the user has
+    // access to it, merge its links in to the parent.
+    if (isset($child['#links']) && empty($child['#printed']) && (!isset($child['#access']) || $child['#access'])) {
+      $element['#links'] += $child['#links'];
+      // Mark the child as having been printed already (so that its links
+      // cannot be mistakenly rendered twice).
+      $child['#printed'] = TRUE;
+    }
+  }
+  return $element;
+}
+
+/**
  * #pre_render callback to append contents in #markup to #children.
  *
  * This needs to be a #pre_render callback, because eventually assigned
diff --git a/modules/blog/blog.module b/modules/blog/blog.module
index d99d6860e..38a6e3655 100644
--- a/modules/blog/blog.module
+++ b/modules/blog/blog.module
@@ -80,11 +80,16 @@ function blog_view($node, $view_mode) {
 function blog_node_view($node, $view_mode) {
   if ($view_mode != 'rss') {
     if ($node->type == 'blog' && (arg(0) != 'blog' || arg(1) != $node->uid)) {
-      $node->content['links']['#links']['blog_usernames_blog'] = array(
+      $links['blog_usernames_blog'] = array(
         'title' => t("!username's blog", array('!username' => format_username($node))),
         'href' => "blog/$node->uid",
         'attributes' => array('title' => t("Read !username's latest blog entries.", array('!username' => format_username($node)))),
       );
+      $node->content['links']['blog'] = array(
+        '#theme' => 'links__node__blog',
+        '#links' => $links,
+        '#attributes' => array('class' => array('links', 'inline')),
+      );
     }
   }
 }
diff --git a/modules/book/book.module b/modules/book/book.module
index fa59a1bd9..a59de8179 100644
--- a/modules/book/book.module
+++ b/modules/book/book.module
@@ -112,7 +112,11 @@ function book_node_view_link($node, $view_mode) {
   }
 
   if (!empty($links)) {
-    $node->content['links']['#links'] = array_merge($node->content['links']['#links'], $links);
+    $node->content['links']['book'] = array(
+      '#theme' => 'links__node__book',
+      '#links' => $links,
+      '#attributes' => array('class' => array('links', 'inline')),
+    );
   }
 }
 
diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index 5334c2728..ea79ea521 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -680,7 +680,11 @@ function comment_node_view($node, $view_mode) {
       $links['comment_forbidden']['html'] = TRUE;
     }
 
-    $node->content['links']['#links'] = array_merge($node->content['links']['#links'], $links);
+    $node->content['links']['comment'] = array(
+      '#theme' => 'links__node__comment',
+      '#links' => $links,
+      '#attributes' => array('class' => array('links', 'inline')),
+    );
 
     // Only append comments when we are building a node on its own node detail
     // page. We compare $node and $page_node to ensure that comments are not
@@ -975,9 +979,14 @@ function comment_build_content($comment, $node, $view_mode = 'full', $langcode =
   entity_prepare_view('comment', array($comment->cid => $comment));
   $comment->content += field_attach_view('comment', $comment, $view_mode, $langcode);
 
+  $comment->content['links'] = array(
+    '#theme' => 'links__comment',
+    '#pre_render' => array('drupal_pre_render_links'),
+    '#attributes' => array('class' => array('links', 'inline')),
+  );
   if (empty($comment->in_preview)) {
     $comment->content['links']['comment'] = array(
-      '#theme' => 'links__comment',
+      '#theme' => 'links__comment__comment',
       '#links' => comment_links($comment, $node),
       '#attributes' => array('class' => array('links', 'inline')),
     );
diff --git a/modules/node/node.module b/modules/node/node.module
index 337c4098d..025be15d3 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -1336,6 +1336,11 @@ function node_build_content($node, $view_mode = 'full', $langcode = NULL) {
   // Always display a read more link on teasers because we have no way
   // to know when a teaser view is different than a full view.
   $links = array();
+  $node->content['links'] = array(
+    '#theme' => 'links__node',
+    '#pre_render' => array('drupal_pre_render_links'),
+    '#attributes' => array('class' => array('links', 'inline')),
+  );
   if ($view_mode == 'teaser') {
     $links['node-readmore'] = array(
       'title' => t('Read more'),
@@ -1343,8 +1348,8 @@ function node_build_content($node, $view_mode = 'full', $langcode = NULL) {
       'attributes' => array('rel' => 'tag', 'title' => strip_tags($node->title))
     );
   }
-  $node->content['links'] = array(
-    '#theme' => 'links__node',
+  $node->content['links']['node'] = array(
+    '#theme' => 'links__node__node',
     '#links' => $links,
     '#attributes' => array('class' => array('links', 'inline')),
   );
diff --git a/modules/rdf/rdf.module b/modules/rdf/rdf.module
index d2790e2ec..a8d62fe22 100644
--- a/modules/rdf/rdf.module
+++ b/modules/rdf/rdf.module
@@ -508,7 +508,7 @@ function rdf_preprocess_node(&$variables) {
   // Adds RDFa markup annotating the number of comments a node has.
   if (isset($variables['node']->comment_count) && !empty($variables['node']->rdf_mapping['comment_count']['predicates'])) {
     // Annotates the 'x comments' link in teaser view.
-    if (isset($variables['content']['links']['#links']['comment-comments'])) {
+    if (isset($variables['content']['links']['comment']['#links']['comment-comments'])) {
       $comment_count_attributes['property'] = $variables['node']->rdf_mapping['comment_count']['predicates'];
       $comment_count_attributes['content'] = $variables['node']->comment_count;
       $comment_count_attributes['datatype'] = $variables['node']->rdf_mapping['comment_count']['datatype'];
@@ -518,7 +518,7 @@ function rdf_preprocess_node(&$variables) {
       // set an empty rel attribute which triggers rule number 5. See
       // http://www.w3.org/TR/rdfa-syntax/#sec_5.5.
       $comment_count_attributes['rel'] = '';
-      $variables['content']['links']['#links']['comment-comments']['attributes'] += $comment_count_attributes;
+      $variables['content']['links']['comment']['#links']['comment-comments']['attributes'] += $comment_count_attributes;
     }
     // In full node view, the number of comments is not displayed by
     // node.tpl.php so it is expressed in RDFa in the <head> tag of the HTML
diff --git a/modules/simpletest/tests/theme.test b/modules/simpletest/tests/theme.test
index c2c9e044a..d7d734bb2 100644
--- a/modules/simpletest/tests/theme.test
+++ b/modules/simpletest/tests/theme.test
@@ -188,6 +188,117 @@ class ThemeItemListUnitTest extends DrupalWebTestCase {
 }
 
 /**
+ * Unit tests for theme_links().
+ */
+class ThemeLinksUnitTest extends DrupalUnitTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => 'Links',
+      'description' => 'Test the theme_links() function and rendering groups of links.',
+      'group' => 'Theme',
+    );
+  }
+
+  /**
+   * Test the use of drupal_pre_render_links() on a nested array of links.
+   */
+  function testDrupalPreRenderLinks() {
+    // Define the base array to be rendered, containing a variety of different
+    // kinds of links.
+    $base_array = array(
+      '#theme' => 'links',
+      '#pre_render' => array('drupal_pre_render_links'),
+      '#links' => array(
+        'parent_link' => array(
+          'title' => 'Parent link original',
+          'href' => 'parent-link-original',
+        ),
+      ),
+      'first_child' => array(
+        '#theme' => 'links',
+        '#links' => array(
+          // This should be rendered if 'first_child' is rendered separately,
+          // but ignored if the parent is being rendered (since it duplicates
+          // one of the parent's links).
+          'parent_link' => array(
+            'title' => 'Parent link copy',
+            'href' => 'parent-link-copy',
+          ),
+          // This should always be rendered.
+          'first_child_link' => array(
+            'title' => 'First child link',
+            'href' => 'first-child-link',
+          ),
+        ),
+      ),
+      // This should always be rendered as part of the parent.
+      'second_child' => array(
+        '#theme' => 'links',
+        '#links' => array(
+          'second_child_link' => array(
+            'title' => 'Second child link',
+            'href' => 'second-child-link',
+          ),
+        ),
+      ),
+      // This should never be rendered, since the user does not have access to
+      // it.
+      'third_child' => array(
+        '#theme' => 'links',
+        '#links' => array(
+          'third_child_link' => array(
+            'title' => 'Third child link',
+            'href' => 'third-child-link',
+          ),
+        ),
+        '#access' => FALSE,
+      ),
+    );
+
+    // Start with a fresh copy of the base array, and try rendering the entire
+    // thing. We expect a single <ul> with appropriate links contained within
+    // it.
+    $render_array = $base_array;
+    $html = drupal_render($render_array);
+    $dom = new DOMDocument();
+    $dom->loadHTML($html);
+    $this->assertEqual($dom->getElementsByTagName('ul')->length, 1, t('One "ul" tag found in the rendered HTML.'));
+    $list_elements = $dom->getElementsByTagName('li');
+    $this->assertEqual($list_elements->length, 3, t('Three "li" tags found in the rendered HTML.'));
+    $this->assertEqual($list_elements->item(0)->nodeValue, 'Parent link original', t('First expected link found.'));
+    $this->assertEqual($list_elements->item(1)->nodeValue, 'First child link', t('Second expected link found.'));
+    $this->assertEqual($list_elements->item(2)->nodeValue, 'Second child link', t('Third expected link found.'));
+    $this->assertIdentical(strpos($html, 'Parent link copy'), FALSE, t('"Parent link copy" link not found.'));
+    $this->assertIdentical(strpos($html, 'Third child link'), FALSE, t('"Third child link" link not found.'));
+
+    // Now render 'first_child', followed by the rest of the links, and make
+    // sure we get two separate <ul>'s with the appropriate links contained
+    // within each.
+    $render_array = $base_array;
+    $child_html = drupal_render($render_array['first_child']);
+    $parent_html = drupal_render($render_array);
+    // First check the child HTML.
+    $dom = new DOMDocument();
+    $dom->loadHTML($child_html);
+    $this->assertEqual($dom->getElementsByTagName('ul')->length, 1, t('One "ul" tag found in the rendered child HTML.'));
+    $list_elements = $dom->getElementsByTagName('li');
+    $this->assertEqual($list_elements->length, 2, t('Two "li" tags found in the rendered child HTML.'));
+    $this->assertEqual($list_elements->item(0)->nodeValue, 'Parent link copy', t('First expected link found.'));
+    $this->assertEqual($list_elements->item(1)->nodeValue, 'First child link', t('Second expected link found.'));
+    // Then check the parent HTML.
+    $dom = new DOMDocument();
+    $dom->loadHTML($parent_html);
+    $this->assertEqual($dom->getElementsByTagName('ul')->length, 1, t('One "ul" tag found in the rendered parent HTML.'));
+    $list_elements = $dom->getElementsByTagName('li');
+    $this->assertEqual($list_elements->length, 2, t('Two "li" tags found in the rendered parent HTML.'));
+    $this->assertEqual($list_elements->item(0)->nodeValue, 'Parent link original', t('First expected link found.'));
+    $this->assertEqual($list_elements->item(1)->nodeValue, 'Second child link', t('Second expected link found.'));
+    $this->assertIdentical(strpos($parent_html, 'First child link'), FALSE, t('"First child link" link not found.'));
+    $this->assertIdentical(strpos($parent_html, 'Third child link'), FALSE, t('"Third child link" link not found.'));
+  }
+}
+
+/**
  * Functional test for initialization of the theme system in hook_init().
  */
 class ThemeHookInitUnitTest extends DrupalWebTestCase {
diff --git a/modules/statistics/statistics.module b/modules/statistics/statistics.module
index 0e093fe5f..39f7e5aef 100644
--- a/modules/statistics/statistics.module
+++ b/modules/statistics/statistics.module
@@ -117,7 +117,12 @@ function statistics_node_view($node, $view_mode) {
     if (user_access('view post access counter')) {
       $statistics = statistics_get($node->nid);
       if ($statistics) {
-        $node->content['links']['#links']['statistics_counter']['title'] = format_plural($statistics['totalcount'], '1 read', '@count reads');
+        $links['statistics_counter']['title'] = format_plural($statistics['totalcount'], '1 read', '@count reads');
+        $node->content['links']['statistics'] = array(
+          '#theme' => 'links__node__statistics',
+          '#links' => $links,
+          '#attributes' => array('class' => array('links', 'inline')),
+        );
       }
     }
   }
diff --git a/modules/translation/translation.module b/modules/translation/translation.module
index 2dc318b70..af295bd20 100644
--- a/modules/translation/translation.module
+++ b/modules/translation/translation.module
@@ -251,7 +251,11 @@ function translation_node_view($node, $view_mode) {
       }
     }
 
-    $node->content['links']['#links'] += $links;
+    $node->content['links']['translation'] = array(
+      '#theme' => 'links__node__translation',
+      '#links' => $links,
+      '#attributes' => array('class' => array('links', 'inline')),
+    );
   }
 }