#1851880: Ensure that formatters for fields with no values are still run added a test that does precisely what its title says:
it ensures formatters still run when a field is empty - and that's true, they run.

Problem is that theme('field') then fails to display the generated output if the field is empty.

in template_preprocess_field():
The following code snippet populates the $items variable that theme_field() / field.tpl will iterate on.
- $element['#items'] contains the field values as found in $entity->field_name
- $element[$delta] contains the output generated by the formatter

  // 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];
    }
  }

"The item render array keys (deltas) should always be a subset of the keys in #items"
This assumption is wrong. The field might be empty, yet the formatter might have returned render elements.

The right logic would be $variables['items'] = element_children($element) here, but as the comment explains, we explicitly avoid element_children() to optimize rendering.

The test added in #1851880: Ensure that formatters for fields with no values are still run passes because the TestFieldEmptyFormatter formatter used by the test does in fact *add* a value if the field is empty - so by the time theme('field') runs, the field is actually not empty.

This is sick, formatters should not add fake items just for display, but this was the only way to make the test pass.
And this is also the reason why ImageFormatterBase currently adds the 'default (fallback) image' as a fake item in the entity before rendering - which is equally sick...

CommentFileSizeAuthor
#1 theme_empty_field-2083835-1.patch2.77 KByched
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Issue summary: View changes

ImageFormatterBase does the same...

yched’s picture

Issue summary: View changes

formatting

yched’s picture

Issue summary: View changes

correct code

yched’s picture

Issue summary: View changes

better explanations

yched’s picture

Status: Active » Needs review
FileSize
2.77 KB

Deltas are supposed to be sequential, so we can just loop on numeric keys in $element starting from 0.

Patch does that, and modifies TestFieldEmptyFormatter to not add items.
- Without the template_preprocess_field() change, this causes a fail in DisplayApiTest
- With the template_preprocess_field() change, DisplayApiTest passes - let's see about the rest of the test suite...

swentel’s picture

Awesome catch. I'll wait until the bot comes back, although I can't think of any problems tbh.

swentel’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

repetition