Change layout templates to use content.region (with BC)

This is an old issue but I was experiencing the exact same issue. I found that it was being caused by a variable name collision when I created a custom template.

For example I had this template, note the "content" region.

custom_onecol:
  label: 'One Column'
  template: layout--one-column
  type: partial
  category: 'Columns: 1'
  default_region: content
  icon_map:
    - [header]
    - [content]
    - [footer]
  regions:
    header:
      label: Header
    content:
      label: Content
    footer:
      label: Footer

When this goes through preprocess function it gets passed like this.

$variables['content']['header'] = [...];
$variables['content']['footer'] = [...]; 
$variables['content']['content'] = [...]; 

When the ds module did its preprocessing it replaced the Drupal core $variables['content']; variable with $variables['content']['content']. This caused all the fields and other properties to immediately get destroyed causing the undefined index error.

  // Copy the regions from 'content' into the top-level.
  foreach (Element::children($variables['content']) as $name) {
    $variables[$name] = $variables['content'][$name];
  }

The quick fix
Don't name your region "content" in your template.

The fix:
I don't think we should be replacing all the variables in $variables['content'] but I also understand that we need to pass up the regions to the top level. I've revised the code a bit so that it doesn't overwrite $variables['content'], however now that region is not being passed. I think that is acceptable given that developers will still have all the variables they need and this should only happen in relative edge cases.

// Copy the regions from 'content' into the top-level.
$content = $variables['content'];
foreach (Element::children($content) as $name) {
  // Do not overwrite any existing variables.
  if (!isset($variables[$name])) {
    $variables[$name] = $content[$name];
  }
}

I ran into some issues with the previous patch. Content would render twice. I've updated the patch, now the "content" variable gets replaced which was the original behavior but without the undefined index notice. I'd like to hear some thoughts, is this the correct direction to go in?

Very interesting! I guess that's why we use 'ds_content' in the one col layout we ship.

But 'content' should work because it's easy to hit this bug; let's see what the bot think. Will definitely work on this and add a test for it!

Ok, looks like the approach is fine. Definitely needs tests though.
I can work on that next week, unless someone beats me to it.

@Swentel, is there a particular approach that you prefer? #3 vs #4? If #3 is the preferred method (which is the one I'm learning towards, but admittedly I'm not an expert on Display Suite), I can look at my use case again and see why it was duplicating the content.

@partdigital Hmm. #3 makes sense because of the isset() check. It's been a while since I looked what's inside $variables['content']. but I think in both cases we probably lose data which might be important further in the process.

I also just looked at how layout_discovery handles this, and the templates use {{content.content}} which makes more sense. This is definitely something which I want to change in 8.x-4.x so we don't copy over all that data anymore.

So maybe we should document that for 8.x-3.x you shouldn't use 'content' as a region and use this issue then to fix it for 8.x-4.x?

Note: if you want to keep on looking, I would appreciate that. I will credit you anyway when I commit the 8.x-4.x part, because without this ping/insight I would probably have never started working on that.

Thanks,

I have the same problem for the fields of a content type.

Notice: Undefined index: "field_name" a ds_preprocess_ds_layout() (línia 618 de /var/www/apps/app0932/src/web/modules/contrib/ds/ds.module)

A validation isset() seems to work.

This is the patch that has worked for me.

@swentel thanks, I will do some more investigating into the #3 approach then and see why it was duplicating content. I should be able to look at this again later this week or early next week.

Moving to right branch.

Complete rewrite of some stuff. The templates now follow the layout pattern: content.{region}

Fixing dependencies

Let's see if we can get this into 8.x-3.x with BC