theme_rdf_metadata doesn't work for most MODULE_preprocess_HOOK()

+++ modules/rdf/rdf.module	2 Feb 2010 19:00:44 -0000
@@ -366,12 +366,14 @@ function rdf_process(&$variables, $hook)
+  // close to that content. The content variable is user_profile in user
+  // profiles, so we check which variable we are processing.
+  $variable_name = !empty($variables['content']) ? 'content' : 'user_profile';

We are hard-coding things. We should check to see if this missing 'content' key is expected or if it's a bug in the user module. What other modules/entity do we need to add to this list? It would be better if we could standardize on 'content', or at least forsee what are the other special cases...

leaving status "needs review" hoping to get feedback on this.

Looks like this is a common pattern in HEAD:

./modules/block/block.admin.inc:541:    $variables['block_listing'][$key] = array();
./modules/comment/comment.module:2191:    $variables['content'][$key] = $variables['elements'][$key];
./modules/node/node.module:1383:    $variables['content'][$key] = $variables['elements'][$key];
./modules/search/search.module:975:      $variables['search'][$key] = drupal_render($variables['form'][$key]);
./modules/user/user.pages.inc:175:    $variables['user_profile'][$key] = $variables['elements'][$key];

We should check whether it would make more sense to standardize on $variables['content'] or if there is a valid reason for having a different key name here (which looks like the hook name).

making title more generic

subscribing for review later.

+++ modules/rdf/rdf.module	3 Feb 2010 12:47:20 -0000
@@ -366,12 +366,15 @@ function rdf_process(&$variables, $hook)
-  // close to that content.
+  // close to that content. The template variable naming scheme varies for
+  // content variables. We check to see whether it is 'content'. If not, we use
+  // the theme hook name.
+  $variable_name = !empty($variables['content']) ? 'content' : $hook;
   if (!empty($variables['rdf_metadata_attributes_array'])) {

This is great. Let's make the test a little more robust by checking that 'content' is both not empty and an array. We should apply the same test to $hook, and if that also fails, then we don't have any render array variable to apply metadata to, so not get into the if block.

Also, what do you think of $content_variable instead of $variable_name?

+++ modules/rdf/rdf.module	3 Feb 2010 12:47:20 -0000
@@ -366,12 +366,15 @@ function rdf_process(&$variables, $hook)
-    if (!isset($variables['content']['#prefix'])) {
-      $variables['content']['#prefix'] = '';
-    }
-    $variables['content']['#prefix'] = theme('rdf_metadata', array('metadata' => $variables['rdf_metadata_attributes_array'])) . $variables['content']['#prefix'];
+      if (!isset($variables[$variable_name]['#prefix'])) {
+        $variables[$variable_name]['#prefix'] = '';
+      }
+      $variables[$variable_name]['#prefix'] = theme('rdf_metadata', array('metadata' => $variables['rdf_metadata_attributes_array'])) . $variables[$variable_name]['#prefix'];

Why different indentation of replacement code?

This review is powered by Dreditor.

Also, if there is a real use-case of user_profile needing metadata, please include that in this patch, along with a test for it, so the fix in this patch gets some test coverage. Thanks!

Awesome. Thanks! Tiny nit:

+++ modules/rdf/rdf.module	12 Mar 2010 10:19:51 -0000
@@ -424,12 +424,20 @@ function rdf_process(&$variables, $hook)
+  else if (!empty($variables[$hook]) && is_array($variables[$hook])) {

s/else if/elseif/

143 critical left. Go review some!

Thanks!

#19: rdf_process-user_702664_19.patch queued for re-testing.

#19: rdf_process-user_702664_19.patch queued for re-testing.

This patch no longer applies. Needs a reroll so moving to 'needs work'.

Reroll against head.

hadn't updated head above patch will fail.