So this could be a "works as designed" situation, but I think solving this would be an improvement to the way that DS Extras finds field templates/functions. I think there this is the same issue mentioned at #1265920-24: The theme's field.tpl.php should be used unless field display settings exist when "Default" option is selected.

I'm using DS Extras to specify a default field template and I'm choosing "Minimal". So all my Display Suite fields are going to use the Minimal field template. For one specific field, field_event_dates, I choose "Drupal Default" as the field template in the Manage Display UI.

When ds_extras_preprocess_field() gets called, I would think that the theme_hook_suggestions should end up like:


These are the "Drupal default" suggestions before ds_extras_preprocess_field() is called.

Instead, this field ends up switching to the minimal field templates, as specified in the ft-default variable (via the DS Extras configuration UI).

I think the correct way that ds_extras_preprocess_field() should handle this situation is to just leave the theme_hook_suggestions alone since I am specifying "Drupal default" as the field template rather than specifying "The configured DS Extras default field template". To put it another way, if you select a default field template (Minimal, Full Reset), there is no other way of getting back to the Drupal default field template for a specific field.



JonMcL’s picture

A rather simple patch attached that possibly solves this issue. Feedback needed and welcome.

swentel’s picture

Status: Active » Needs review

Hmm this makes very much sense indeed, setting to needs review so I can let it test next week.

jyve’s picture

Status: Needs review » Reviewed & tested by the community

Patch tested, and makes perfect sense!

swentel’s picture

Version: 7.x-2.x-dev » 8.x-2.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

committed and pushed on 7.x-2.x and 7.x-1.x, thanks!
Moving to 8.x-2.x.

swentel’s picture

Status: Patch (to be ported) » Closed (fixed)

And committed and pushed to 8.x-2.x branch as well.

donquixote’s picture

+  elseif (!isset($config['func']) || empty($config['func'])) {

It is enough to just check if (empty($config['func'])). empty() implies !isset(), and will not break on undefined variables.

donquixote’s picture

Also, if we test empty() there, we should also test empty() in the first condition, like so:

  // Determine the field template. In case it's something different
  // than theme_field, we'll add that function as a suggestion.
-  if (isset($config['func']) && $config['func'] != 'theme_field') {
+  if (!empty($config['func']) && $config['func'] != 'theme_field') {

If $config['func'] has any value that PHP considers empty, we don't want to build functions with it. Right?
Or we only need isset() in both cases.

The existence of the first if () tells us that the only relevant case to exclude down in this elseif () block is where $config['func'] === 'theme_field'.

donquixote’s picture

And a different problem:

In the second block with $field_instance_info['ds_extras_field_template'], we also don't want to overwrite $config['func'] === 'theme_field', do we? So the applied patch / commit is incomplete.

Fixing all this also allows to simplify the code a lot.
Maybe I will open a new issue for this.

donquixote’s picture

And in the second block (condition broken to multiple lines):

  // Check if we have a default field template on instance level.
  elseif (isset($field_instance_info['ds_extras_field_template'])
    && !empty($field_instance_info['ds_extras_field_template'])
    && $field_instance_info['ds_extras_field_template'] != 'theme_field'
  ) {
  // Use Display Suite Extras Default theming function.
  elseif (!isset($config['func']) || empty($config['func'])) {

In the case of $field_instance_info['ds_extras_field_template'] === 'theme_field', it should not fall through to the next condition, but rather just stop altogether.

donquixote’s picture