copying field.tpl.php into a custom theme crashes your site

[jenlampton]

I just got a nasty fatal error when I copied field.tpl.php from core/modules/field/templates/field.tpl.php into my theme:

Recoverable fatal error: Argument 1 passed to backdrop_attributes() must be of the type array, null given, called in /backdrop/themes/boots/templates/field.tpl.php on line 57 and defined in backdrop_attributes() (line 2613 of /Users/jlampton/Sites/boots/backdrop/html/core/includes/common.inc).

It looks like the problem is that $item_attributes doesn't exist. It's never set in template_preprocess_field, or anywhere else that I can see. Is it supposed to be? If not, why was it ever in here?

[quicksketch]

The PR looks good to fix the problem but it causes a difference from the theme_field() function. Both the template and the theme function should have identical output.

In theme_field(), we have this bit:

    $item_attributes = (isset($variables['item_attributes'][$delta])) ? backdrop_attributes($variables['item_attributes'][$delta]) : '';

So instead of just deleting $item_attributes in the field.tpl.php, we should change it to:

<?php print isset($variables['item_attributes'][$delta])) ? backdrop_attributes($variables['item_attributes'][$delta] : ''; ?>

It's not pretty but it matches the existing code. If we want to clean this up, we can add template_preprocess_field() to ensure that $variables['item_attributes'][$delta] is set for all fields.

[jenlampton]

I'm not seeing any difference from theme_field() and field.tpl.php because item attributes per delta are never set. Why print something out so carefully if it doesn't exist? Shouldn't we be deleting it in both places instead?

[jenlampton]

I applied your suggestion in backdrop/backdrop#1120, but I think it makes the template file (and theme function) unnecessarily cumbersome in the name of flexibility that's never used. So I've created #1352 :)

[quicksketch]

Why print something out so carefully if it doesn't exist? Shouldn't we be deleting it in both places instead?

Because it was supported in Drupal 7. It looks like RDFa module had used it in the past:

      $variables['item_attributes_array'][$delta] = rdf_rdfa_attributes($mapping[$field_name], $item);

And although we don't have RDFa in core any more, removing variables that had been there previously seems like a regression we don't need to create.

However I totally agree that this is ugly and messy. I would suggest that we create a preprocess hook and populate these variables so we don't have to check for their existence.

[quicksketch]

It looks like the need to check for all these variables is a remanent of removing template_process_fields(), which defined all of these:

https://api.drupal.org/api/drupal/modules!field!field.module/function/template_process_field/7

We should just ensure each of these variables is populated with at least an empty array in our existing template_preprocess_field().

It looks like $title_attributes was also (possibly inadvertently?) removed in the change.

[jenlampton]

New PR takes a completely different approach, and defines variables for all the attributes as @quicksketch recommends.

[quicksketch]

Super, looks great. Merged backdrop/backdrop#1191 into 1.x and 1.2.x.