Undefined array key "view_mode" in block_content_theme_suggestions_block_alter

Possible to get test coverage around this issue?

Made a small change directly. Don't think we need to mention the ticket in the comment since git blame can show that. But did leave the comment that this is solving a php warning.

Test-only failure does indeed show coverage and rest of the changes appear fine.

The steps to reproduce say:

Add a field block or extra field block to a layout builder enabled page. Then view the page.

But the function does this:

function block_content_theme_suggestions_block_alter(array &$suggestions, array $variables) {
  $suggestions_new = [];
  $content = $variables['elements']['content'];

  $block_content = $variables['elements']['content']['#block_content'] ?? NULL;

  if ($block_content instanceof BlockContentInterface) {

So it shouldn't run for field or extra field blocks at all, and I don't see how a block content block can be rendered without a view mode set.

The test coverage is quite low-level so doesn't confirm the steps to reproduce either. It's not clear to me how we end up in this situation, and if so, whether the test is fixing the right thing or not - if this alter is running for the wrong blocks, then that's the thing that should be changed.

I am also seeing this issue when using the Fixed Block Content contrib module.

The module renders a block_content entity directly as its only content, the same situation as described by OP. This situation does seem perfectly reasonable to me. There's any number of reasons why a block, e.g. a custom block, might only directly render a block_content entity as its content.

MR !9426 attempts to resolve this issue by looking to $variables['elements']['content']['#view_mode'] to determine the view mode of the block_content entity rather than $variables['elements']['#configuration']['view_mode'].

Hide patches since we are now using MRs.

There are now 2 MRs up with different approaches and 1 not having test coverage.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

False positive

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

MR updated + missing type hints.

Believe feedback on this one has been addressed.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Yes, they should be, but wow do you guys get an award for confusing module / hook names.

At first glance I thought block_content_theme_suggestions_test_entity_extra_field_info was theme suggestion until I saw the end.

Theme suggestions are not converted yet, but to be clear all three hooks in this module should be able to be converted.

Refactor for move to hooks seems good.

Left some questions on the MR - thanks

Think this is ready for a review again.

Believe #34 is still needed no?

For theme suggestions for inline blocks? Is that in scope of this issue? That was a response to "Does this break Inline blocks?" Which I've manually confirmed it doesn't. I think if we want additional test coverage there it should be a separate issue?

Think I would agree with that. Not opening up the follow up unless we want the additional coverage.

Here's a code-only patch for use with composer and 10.5.

patch for 10.3.x

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Committed 5de749b and pushed to 11.x. Thanks!

adding a patch for 11.1.x since this has not made it into a release yet.

Asked the other release managers about the duplicate isssue, #3551116: strtr(): Passing null to parameter #1 ($string) of type string is deprecated in block_content_theme_suggestions_block_alter(). longwave said that this should be backported.

The diff here still applies to 11.1.x. Re-opening for am MR for Drupal 10.

This needs an MR for Drupal 10.

Backported https://git.drupalcode.org/project/drupal/-/commit/5de749b83a9ee2d2af0c1... to 10.6.x, swapping all OOP hooks in block_content_theme_suggestions_test to the procedural equivalent.

Seems like a good backport.

Committed and pushed 9839653ed9a to 10.6.x. Thanks!

Here is a patch that is 10.6 compatible :

diff --git a/core/modules/block_content/block_content.module b/core/modules/block_content/block_content.module
--- a/core/modules/block_content/block_content.module
+++ b/core/modules/block_content/block_content.module	(date 1779354874139)
@@ -192,21 +192,25 @@
  */
 function block_content_theme_suggestions_block_alter(array &$suggestions, array $variables) {
   $suggestions_new = [];
-  $content = $variables['elements']['content'];
+  $content = $variables['elements']['content'] ?? [];

-  $block_content = $variables['elements']['content']['#block_content'] ?? NULL;
+  $block_content = $content['#block_content'] ?? NULL;

   if ($block_content instanceof BlockContentInterface) {
-    $bundle = $content['#block_content']->bundle();
-    $view_mode = strtr($variables['elements']['content']['#view_mode'], '.', '_');
-
-    $suggestions_new[] = 'block__block_content__view__' . $view_mode;
+    $bundle = $block_content->bundle();
     $suggestions_new[] = 'block__block_content__type__' . $bundle;
-    $suggestions_new[] = 'block__block_content__view_type__' . $bundle . '__' . $view_mode;
+
+    $view_mode = strtr($content['#view_mode'] ?? '', '.', '_');
+    if ($view_mode) {
+      $suggestions_new[] = 'block__block_content__view__' . $view_mode;
+      $suggestions_new[] = 'block__block_content__view_type__' . $bundle . '__' . $view_mode;
+    }

     if (!empty($variables['elements']['#id'])) {
       $suggestions_new[] = 'block__block_content__id__' . $variables['elements']['#id'];
-      $suggestions_new[] = 'block__block_content__id_view__' . $variables['elements']['#id'] . '__' . $view_mode;
+      if ($view_mode) {
+        $suggestions_new[] = 'block__block_content__id_view__' . $variables['elements']['#id'] . '__' . $view_mode;
+      }
     }

     // Remove duplicate block__block_content.