'base hook' key prevents template suggestions from working

As much as 'base hook' should 100% work, I'm also in favor of going with this.

The "fix" for the fail in #5 seems bad.

diff --git a/core/modules/layout_discovery/tests/src/Kernel/LayoutTest.php b/core/modules/layout_discovery/tests/src/Kernel/LayoutTest.php
index 9d4c446..7870b81 100644
--- a/core/modules/layout_discovery/tests/src/Kernel/LayoutTest.php
+++ b/core/modules/layout_discovery/tests/src/Kernel/LayoutTest.php
@@ -135,7 +135,7 @@ public function renderLayoutData() {
     ];
 
     $data['layout_onecol'][] = <<<'EOD'
-<div data-drupal-selector="edit-layout" class="layout--onecol">
+<div class="layout--onecol">
   <div class="layout-region layout-region--content">
     This is the content
   </div>

What about this?

The title of this issue isn't specific enough.

+++ b/core/modules/field_layout/tests/modules/field_layout_test/field_layout_test.module
@@ -0,0 +1,15 @@
+  if (isset($_GET['test_layout_theme_suggestion']) && $hook == 'layout__twocol') {

The only reason this test is failing is because we're checking $hook, but per the docs for hook_theme_suggestions_alter(), what we really need to check in this case is $variables['theme_hook_original']. When I make that change, this test passes. See attached patch.

I'm starting to wonder whether we simply can't get behind the fact that ds layouts aren't prefixed with 'layout--' (if I do that, switching happens ...)

I suspect there might still be a core bug if the specific layout and/or suggestion isn't prefixed with 'layout--'. But that's not what this issue title, summary, or test in patch is currently about. Do we want to repurpose this issue to be about that, or is there still some bug with layouts/suggestions that even are prefixed as such that needs to be addressed first?

#15's retest passed, so setting to "maintainer needs more info" for figuring out how we want to proceed with this issue, per #15.

I've tried #15 with configuration described here: https://www.drupal.org/node/2867794
but it doesn't work. Drupal returns:

Twig_Error_Loader: Template "themes/bootstrap/templates/bs-2col.html.twig" is not defined (Drupal\Core\Template\Loader\ThemeRegistryLoader: Unable to find template "themes/bootstrap/templates/bs-2col.html.twig" in the Drupal theme registry.) in "modules/ds/templates/ds-entity-view.html.twig" at line 10. in Twig_Loader_Chain->getCacheKey() (line 115 of vendor/twig/twig/lib/Twig/Loader/Chain.php).

@sedax this isn't related with core this is a problem with boostrap theme https://www.drupal.org/node/2871551

@fastangel, the Bootstrap base theme has nothing to do with this issue or @sedax's "issue".

edit: I was wrong

Sorry @markcarver I forgot to tell. The error I comment is about the @sedax's issue :)

Sorry, in DS issues tell me that the problem is core, here tell me it's bootstrap, i don't know what patches try XD

This is really a quite big issue. Websites working well with 8.2 are forced to update to 8.3 (to get security patches and bug fixes) and when they do if you use layout suggestions templates they won't work anymore.

DS got a workaround mentioned on #13, which so far only works if on your layout you specify the class '\Drupal\ds\Plugin\DsLayout'. The workaround makes its best to fix this issue for DS users (well done), but from my understanding specifying the DsLayout class creates an unneeded dependency?

Solution suggested on #2 - #5 fixes the problem, although from what I can see is not (yet) the right direction(?).
Also #15 passes. But isn't hat supposed to fail?

I answer my own comment:

Also #15 passes. But isn't hat supposed to fail?

According to this IS is supposed to fail, but it doesn't. As @effulgentsia explained in #15:

I'm starting to wonder whether we simply can't get behind the fact that ds layouts aren't prefixed with 'layout--' (if I do that, switching happens ...)

I suspect there might still be a core bug if the specific layout and/or suggestion isn't prefixed with 'layout--'. But that's not what this issue title, summary, or test in patch is currently about. Do we want to repurpose this issue to be about that, or is there still some bug with layouts/suggestions that even are prefixed as such that needs to be addressed first?

So I confirm there is an issue with layouts not being prefixed with layout--.
I succesfuly load my suggestions if I prefix both my layout definition with layout__ and it's (base) template with layout--, i.e.:

layout__onecol_hero:
  label: One column layout with hero (fixed)
  template: templates/layouts/layout--onecol-hero
  regions:
    hero:
      label: Hero
    main:
      label: Main
    footer:
      label: Footer

And of course all .twig templates prefixed with layout-- as well.

What's interesting is the layout defined on #15 test is with one underscore, but that doesn't work for me. I must use 2 underscores.

Worth mentioning I've only tested this against DS. I'll give a try against Panels as well and if we confirm the issue is with layout prefix we can change the IS (or open a new issue, as this one already looks messy).

@gambry did you manage to give it a try for panels?

Panels is slightly different, as - at least on my tests - you don't have suggestions and you have to create your own either with hook_theme_suggestions_alter() or hook_theme_suggestions_HOOK_alter() on mytheme.theme, where HOOK is the bundle name of your layout with layout_plugin and the base hook 'layout' with layout_discovery + core layout subsystem.

I managed to get my suggestions working just updating mytheme.theme from:

function mytheme_theme_suggestions_onecol_hero_alter(array &$suggestions, array $variables, $hook) {
  $suggestions[] = 'onecol_hero__front';
}

to

function mytheme_theme_suggestions_layout_alter(array &$suggestions, array $variables, $hook) {
  if($variables['theme_hook_original'] == 'onecol_hero') {
    $suggestions[] = 'layout__onecol_hero__front'';
  }
}

and renaming the file template suggestion from onecol-hero--front to layout--onecole-hero--front.

No other changes were needed.

So I confirm there is an issue with layouts not being prefixed with layout--.

Instead of that I would say there is an inconsistency on how layout templates and suggestions are loaded.

On 8.3 with layout discovery and this layout:

onecol_hero:
  label: One column layout with hero (fixed)
  template: templates/layouts/onecol-hero
  regions:
    hero:
      label: Hero
    main:
      label: Main
    footer:
      label: Footer

• Base template templates/layouts/onecol-hero.html.twig loads correctly.
• Suggesting onecol_hero__front does NOT load templates/layouts/onecol-hero--front.html.twig is not loaded.
• Suggesting layout__onecol_hero__front DOES load templates/layouts/layout--onecol-hero--front.html.twig

Can anyone confirm this behaviour?

Here's what I'm seeing.

I want to preprocess a layout with the name foobar.

my_module_preprocess_layout__foobar() should work.

Currently it does not.

I believe this is because of the regex in Registry::postProcessExtension():
/^{$prefix}_preprocess_(((?:[^_]++|_(?!_))+)__.*)/
This very complex (and slightly malformed) regex would have the following matches:
0 => my_module_preprocess_layout__foobar
1 => layout__foobar
2 => layout

It then checks the existing theme hooks for 'layout__foobar', which doesn't exist.

If we switch the regex to /^{$prefix}_preprocess_(((?:[^_]++|_(?!_))+)__(.*))/, then we have the following matches:
0 => my_module_preprocess_layout__foobar
1 => layout__foobar
2 => layout
3 => foobar

Then if we check for 'foobar' and fallback to checking for 'layout__foobar'.
This allows the expected behavior to finally occur!

I'm not actually sure that I'm thinking of the same issue anymore. This was originally about template suggestions, not preprocess suggestions...
Might have to make a new issue.

Added docs so no one else has to guess what that regex does.
This patch adds the final $matches[3] by adding another set of parentheses.

I think my comments from #30 onward are really better served as a separate followup to #2559825: Preprocess functions from base hooks not triggered for theme hooks not using the __ pattern, restoring this issue.

#2886187: Document the regex in Registry::postProcessExtension() used to find preprocess functions for that other issue.

Can this issue be closed, was it resolved in the follow-up issues? The issue summary is a bit lacking (as it mentions)

@joelpittet bug it's still there AFAIK, but I would rather open a new issue instead keeping this one open.

The starting point of the new issue should be #15 "I suspect there might still be a core bug if the specific layout and/or suggestion isn't prefixed with 'layout--'". There is already a bit of investigation from #25 to #29.

Will create the new issue, post the link here and the close this in favour of the other one.

Closing this in favour of https://www.drupal.org/node/2911996