Problem/Motivation

PageBlockDisplayVariant::__construct() type-hints the concrete class Drupal\Core\Block\BlockManager for
$block_manager (src/Plugin/DisplayVariant/PageBlockDisplayVariant.php, line 67).

A module that decorates the plugin.manager.block service replaces it with a class that implements BlockManagerInterface
but does not extend BlockManager. Page Manager then fatals every time it instantiates a Block page variant:

TypeError: Drupal\page_manager\Plugin\DisplayVariant\PageBlockDisplayVariant::__construct():
  Argument #8 ($block_manager) must be of type Drupal\Core\Block\BlockManager,
  Drupal\canvas\Block\BlockManagerDecorator given, called in
  page_manager/src/Plugin/DisplayVariant/PageBlockDisplayVariant.php on line 77

Drupal Canvas decorates plugin.manager.block this way
(Drupal\canvas\Block\BlockManagerDecorator). Any other module that decorates the service triggers the same error.

Existing pages break, not only new ones. A Block page variant created before the decorating module was enabled fails as soon as the module
is enabled:

Before After enabling the decorating module
Public page using the variant 200 500
Page's edit screen (/admin/structure/page_manager/manage/{page}/general) 200 500
Variant's content screen 200 500
Adding a new Block page variant works 500

Site builders can't fix the affected pages through the UI, because their edit screens also fail.

The narrower type isn't needed:

  • The parent class, CTools' BlockDisplayVariant::__construct(), already accepts BlockManagerInterface $block_manager.
  • PageBlockDisplayVariant only passes $block_manager on to the parent. It calls no BlockManager-only
    methods.
  • Page Manager already depends on the interface elsewhere, for example
    PageManagerController::__construct(BlockManagerInterface $block_manager,
      …)

    .

finding-pm-block-display-typeerror

Steps to reproduce

  1. Install Drupal 11 with Page Manager 8.x-4.x (page_manager_ui) and CTools 4.1.x.
  2. At /admin/structure/page_manager, add a page with a Block page variant and visit the page. It returns 200.
  3. Enable a module that decorates plugin.manager.block, for example Drupal Canvas 1.x.
  4. Visit the page again. It returns 500 with the TypeError above.
  5. Open the page's edit screen, or click Add variant → type Block page → Next. Both return
    500.

Reproduced with Drupal 11.4.8, PHP 8.4, Page Manager 8.x-4.0, CTools 4.1.1, and Drupal Canvas 1.x.

Proposed resolution

Type-hint the interface, matching the parent class:

-use Drupal\Core\Block\BlockManager;
  +use Drupal\Core\Block\BlockManagerInterface;
  …
  -   * @param \Drupal\Core\Block\BlockManager $block_manager
  +   * @param \Drupal\Core\Block\BlockManagerInterface $block_manager
  …
  -  public function __construct(…, Token $token, BlockManager $block_manager, ConditionManager $condition_manager, ModuleHandlerInterface
  $module_handler) {
  +  public function __construct(…, Token $token, BlockManagerInterface $block_manager, ConditionManager $condition_manager, ModuleHandlerInterface
  $module_handler) {

This widens the accepted type, so existing callers keep working. PHP does not apply signature-compatibility rules to constructors, so subclasses
that override the constructor are not affected.

Remaining tasks

  • MR with the change above
  • Test coverage: construct PageBlockDisplayVariant with a BlockManagerInterface implementation that does not extend
    BlockManager. The existing PageBlockDisplayVariantTest only prophesizes the concrete BlockManager.
  • Review

API changes

PageBlockDisplayVariant::__construct() accepts BlockManagerInterface instead of BlockManager. The type is
wider, so this change is backward compatible.

CommentFileSizeAuthor
#2 finding-pm-block-display-typeerror.png220.23 KBrealsp
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

realsp created an issue. See original summary.

realsp’s picture

Issue summary: View changes
StatusFileSize
new220.23 KB

joelpittet’s picture

Assigned: realsp » Unassigned
Priority: Normal » Major
Status: Active » Fixed

The AI should learn to be a bit less verbose, but the fix is easy enough to fix

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

joelpittet’s picture

Thanks, the fix is correct. The parent class already accepts BlockManagerInterface, and the tests now fail without the change. Committing.

Thanks also for disclosing the AI assistance. One request for next time: please keep the issue summary in proportion to the change. This is a one-line type-hint fix, but the summary took a while to read. The error message, the steps to reproduce and the proposed resolution would have been enough; the before/after table, the API-change analysis and the diff repeated in the summary weren't needed. Shorter summaries are quicker for maintainers to review.

  • joelpittet committed 2d5588f6 on 8.x-4.x authored by realsp
    task: #3627021 Accept BlockManagerInterface in PageBlockDisplayVariant...