Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Using 'base hook' in the theme hooks doesn't find theme suggestions (e.g. layout--onecol -> layout--onecol--node).
This has been a struggle for Display suite, see #2802429: Display layout twig not overridable..
A work around is to explicitly define 'template_preprocess_layout' in the preprocess functions key.
(needs a better issue summary)
Also, wondering whether this qualifies for 8.x-3.x directly since it's quite the annoying bug.
Comment | File | Size | Author |
---|---|---|---|
#33 | 2862683-layout-new_fix-33-interdiff.txt | 1.34 KB | tim.plunkett |
#33 | 2862683-layout-new_fix-33.patch | 3.18 KB | tim.plunkett |
#30 | 2862683-layout-30-PASS.patch | 2.66 KB | tim.plunkett |
#30 | 2862683-layout-30-FAIL.patch | 1.05 KB | tim.plunkett |
#15 | interdiff-8-15.txt | 780 bytes | effulgentsia |
Comments
Comment #2
swentel CreditAttribution: swentel as a volunteer commentedComment #3
swentel CreditAttribution: swentel as a volunteer commentedComment #4
swentel CreditAttribution: swentel as a volunteer commentedComment #5
tim.plunkettAs much as 'base hook' should 100% work, I'm also in favor of going with this.
Comment #8
swentel CreditAttribution: swentel as a volunteer commentedfailing test - might be a bit weird, but my head is almost frozen ..
looking at the diff, I'm not even sure whether it even explains the problem nicely as the suggestion alter hook code is kind of cheating in a way
going to bed now.
Comment #9
tim.plunkettThe "fix" for the fail in #5 seems bad.
Comment #11
swentel CreditAttribution: swentel as a volunteer commentedSo what I'm seeing is that 'data-drupal-selector="edit-layout"' is gone, the wrapper is still there with the layout--onecol class on .. not sure exactly where that selector is set atm or why it's needed.
Comment #12
swentel CreditAttribution: swentel as a volunteer commentedLooked at other theme functions using 'base hook' in core and there are a few, like field, block, comment
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 ...)
Comment #13
swentel CreditAttribution: swentel as a volunteer commentedSo, after stepping through the discovery mechanism, I think I figured out why this isn't working, unless I'm missing something.
Because layout hooks have 'base hook' key set explicitly, suggestions will not be registered.
Now, AFAICS, this isn't a problem of the layout module (as you already mentioned somewhere else), but a bug in the theme discovery code, so I think we can close this one.
Not sure about a proper fix though, but I've made an insane workaround for DS, see https://www.drupal.org/node/2802429#comment-12009414 - so I can live with that for now :)
Comment #14
tim.plunkettWhat about this?
Comment #15
effulgentsia CreditAttribution: effulgentsia at Acquia commentedThe title of this issue isn't specific enough.
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 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?Comment #17
effulgentsia CreditAttribution: effulgentsia at Acquia commented#15's retest passed, so setting to "maintainer needs more info" for figuring out how we want to proceed with this issue, per #15.
Comment #18
csedax90 CreditAttribution: csedax90 commentedI'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).
Comment #19
csedax90 CreditAttribution: csedax90 commentedComment #20
fastangel CreditAttribution: fastangel as a volunteer commented@sedax this isn't related with core this is a problem with boostrap theme https://www.drupal.org/node/2871551
Comment #21
fastangel CreditAttribution: fastangel as a volunteer commentedComment #22
markhalliwell@fastangel, the Bootstrap base theme has nothing to do with this issue or @sedax's "issue".edit: I was wrong
Comment #23
fastangel CreditAttribution: fastangel as a volunteer and at Desa 4 commentedSorry @markcarver I forgot to tell. The error I comment is about the @sedax's issue :)
Comment #24
csedax90 CreditAttribution: csedax90 commentedSorry, in DS issues tell me that the problem is core, here tell me it's bootstrap, i don't know what patches try XD
Comment #25
gambryThis 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?
Comment #26
gambryI answer my own comment:
According to this IS is supposed to fail, but it doesn't. As @effulgentsia explained in #15:
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 withlayout--
, i.e.: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).Comment #27
tassos CreditAttribution: tassos commented@gambry did you manage to give it a try for panels?
Comment #28
gambryPanels 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()
orhook_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:
to
and renaming the file template suggestion from
onecol-hero--front
tolayout--onecole-hero--front
.No other changes were needed.
Comment #29
gambryInstead 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:
templates/layouts/onecol-hero.html.twig
loads correctly.onecol_hero__front
does NOT loadtemplates/layouts/onecol-hero--front.html.twig
is not loaded.layout__onecol_hero__front
DOES loadtemplates/layouts/layout--onecol-hero--front.html.twig
Can anyone confirm this behaviour?
Comment #30
tim.plunkettHere'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!
Comment #31
tim.plunkettComment #33
tim.plunkettI'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.
Comment #34
tim.plunkettI 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.
Comment #35
tim.plunkett#2886187: Document the regex in Registry::postProcessExtension() used to find preprocess functions for that other issue.
Comment #37
joelpittetCan this issue be closed, was it resolved in the follow-up issues? The issue summary is a bit lacking (as it mentions)
Comment #38
gambry@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.
Comment #39
gambryClosing this in favour of https://www.drupal.org/node/2911996