Invalid argument supplied for foreach() in ds_preprocess_ds_layout() line 366 of ds.module

Is layout_plugin still enabled or not ?

Because DS 8.x-3.x is dependant on core 8.3.x which includes the layout discovery module now and not compatible anymore with contrib layout_plugin.

Other than that, still need the upgrade hook for it anyway, so we could potentially use this issue for it. The patch itself seems weird as this works fine (at least in tests).

That's weird .. I don't get why DS core isn't failing on this. I have a feeling something external is messing with those settings.
Let's put the patch on needs review and see what the bot thinks.

So , the entities fail test is probably due to something unrelated.

But other than than that, the patch seems to be fine. I'm baffled :)
Has something changed the last 10 hours in core or so ? :)

@samerali So I've been wondering: is this a custom layout ?
It would be interesting also to know which class it's using in case it's custom (e.g. if you check ds.layouts.yml, all our layouts classes are \Drupal\ds\Plugin\DsLayout. Maybe that might give me a clue where the difference is coming from.

And on which entity type/bundle is it applied ?

Also checking template_preprocess_layout() in layout_discovery, that is actually using the check you are doing in the patch and assigning it to $variable['settings'] (which then in ds_preprocess_layout is picked up in most cases)

As mentioned, this means that template_preprocess_layout() is not being run at the correct time, or at all.
Can you post your *.layouts.yml entry?

We are experiencing the same error, can you review the code we are using?

In our *.layouts.yml

colli_teaser:
  label: Colli Teaser
  category: Display Suite
  class: '\Drupal\ds\Plugin\DsLayout'
  type: partial
  template: templates/layout/colli-teaser
  regions:
    left:
      label: Left
    right:
      label: Right
    bottom:
      label: Bottom

The template

<{{ outer_wrapper }}{{ attributes.addClass('node--type-colli', 'node--view-mode-teaser') }}>

    {{ title_suffix.contextual_links }}

    <div class="teaser-table">
        <div class="teaser-table-row">
            <{{ left_wrapper }}{{ left_attributes.addClass('group-left') }}>
                {{ left }}
            </{{ left_wrapper }}>

            <{{ right_wrapper }}{{ right_attributes.addClass('group-right') }}>
                {{ right }}
            </{{ right_wrapper }}>
        </div>
    </div>

    <{{ bottom_wrapper }}{{ bottom_attributes.addClass('group-bottom') }}>
        {{ bottom }}
    </{{ bottom_wrapper }}>
</{{ outer_wrapper }}>

The above code is added in a custom theme.

Oh, I think there's an issue somewhere that involves the registration of custom layouts in themes iirc, but can't find it right now, maybe that's the key.

So interestingly enough, it seems the one column layout has this problem too, this is very weird to say at the least.

I noticed all the fails are missing a library key...
Could that be the problem?

Not sure, but it's the first interesting clue to start debugging in a few days :)

Let's see what happens if entities test uses ds_1col (which has no library key)

I'm with stupid today, sorry .. one second

remove library key from stacked

Can we add a test for the 1col case first so that we have a failing test at least?

https://www.drupal.org/node/2857972 sounds also related...

#2857972: Some content wrappers are not passed to display suite layouts overridden twig templates is simply about the wrapper not appearing, but I'm not getting the notices on my local machine, so we're back to square one regarding that.

Try to use the template we made in a test theme. Maybe that Will trigger a fail. The linked issue is also about layout in a theme and we had missing wrappers too, the above patch fixed everything at once but is obviously wrong.

It's a core bug, and it's definitely related to it being defined by a theme.

Okay here's what I'm seeing:
In \Drupal\Core\Theme\Registry::build() there are a bunch of calls to $this->processExtension() in a row.
During the first round it loops through each module, and once it gets to layout_discovery, that loads the theme implementation for each entry in every *.layouts.yml
Then it goes through base themes and stuff, which isn't relevant to this.

Then it calls it for the 'theme_engine' and for the 'theme' itself.

Of course, the theme doesn't have the layout in its hook_theme().

But the theme_engine is the problem.
It calls twig_theme(), which scans for *any* *.html.twig in a theme. It finds the colli-teaser.html.twig, and creates a dummy theme definition, and then uses that to overwrite the valid one.

This isn't unique to layouts or DS. I'll open a core issue for this later. But in the meantime, themes cannot safely provide layout definitions, unless they alter it with hook_theme_registry_alter() or provide a hook_theme() to add in the 'base hook' => 'layout'

In that case DS should add a theme registry alter workaround. I don't want to piss off the DS user base :). Sadly enough that workaround need to be copied to panels as well. Can't we have a workaround in the core module before 8.3 is releases? :S

And offcourse add a test theme.

Working on a core issue here #2861840: Preprocess functions are not merged when a module registers a theme hook for a theme-provided template

This should work? Still probably want a test though, things shouldn't break this bad.

Ok, with tests.

Thinks Tim for figuring this out!

Slightly better

committed and pushed

Oops wrong issue ... deleted my comment