Preprocess functions are not merged when a module registers a theme hook for a theme-provided template

Looking fine, will test this tomorrow.

I tested this on our production site and it works perfectly.
Code and test looks good as well.

+++ b/core/lib/Drupal/Core/Theme/Registry.php
@@ -565,10 +566,20 @@ protected function processExtension(array &$cache, $name, $type, $theme, $path)
+        // Before merging the new result with the original information, ensure
+        // that the final result does not end up with both a template and a
+        // function defined.

I think the documentation here could be improved. Could we state more clearly why we want to avoid both being defined in the theme registry?

We should also create some test coverage outside the layout_discovery module since this is a bug in the theme system.

Seems like both points are covered.
Second try for RTBC.

The bug fix itself looks good now. Thank you also for clarifying the documentation, it looks good for me now. I was manually testing this and added test coverage to ensure that multiple preprocess functions are added and ordered correctly since that was one of the first things I wanted to test manually and was still lacking test coverage.

I looked back to the history of this line and it looks like this bug exists in Drupal 7 as well and could be backported there later.

The minor BC break being caused by the change needs sign-off from another maintainer.

The proposed solution and test look great. I'll +1 this from another theme system maintainer's point of view.

This one hunk looked a bit suspect could you explain why this is needed?

+++ b/core/lib/Drupal/Core/Theme/Registry.php
@@ -565,10 +566,18 @@ protected function processExtension(array &$cache, $name, $type, $theme, $path)
+        // If a theme implementation definition provides both 'template' and
+        // 'function', the 'function' will be used. In this case, if the new
+        // result provides a 'template' value, any existing 'function' value
+        // must be removed for the override to be called.
+        if (isset($result[$hook]['template'])) {
+          unset($cache[$hook]['function']);
+        }

Not sure I understand why this was needed. Could you add this to the Proposed solution in the IS?

DS implemented a workaround but other layout modules are probably being hit by the same bug. Joel, you should ask tim for more information if you need some.

but other layout modules are probably being hit by the same bug

Yes, I can confirm that this is happening in other layout modules #2872583: Empty "layout" and "settings" in _bootstrap_layouts_preprocess_layout().

Related core issue (or possibly a dup of this issue?).

+1 on this so far after some initial testing.

I'd been 'fighting' for a while why I couldn't get omega_preprocess_layout() to be fired.
I've been working on an integration (currently template based and not custom class) of layout_discovery into the 5.x branch of Omega, and getting some final logic in preprocess_layout is the last bit I need to get all features for responsive region management in order to assign some final classes based on populated regions.

Will be running this patch locally for testing, and looking more over the DS solution so that I don't need to require Omega users to patch Drupal for this functionality until this is in core.

Definitely have some additional investigation/discovery to do around this issue and #2862683: 'base hook' key prevents template suggestions from working.

I came here from this issue:
#2875775: Overriding template results in fatal error, id() on null

Patch from #17 seems to solve the problem.

Patch from #17 works after latest update to Drupal 8.4.4, too.
Does this make it RTBC?

Committed as 0f2bebf and pushed to 8.5.x

As per https://www.drupal.org/core/release-cycle-overview#current-development-c... 8.4 is in 'critical fixes only', so this can't be backported to 8.4