Problem/Motivation

When using Layout Library with Layout Builder in Drupal 11, clicking
“Discard changes” on a saved layout library item can cause a fatal error:
Call to a member function label() on null in DiscardLayoutChangesForm::getDescription().
The form expects an entity context, but SectionStorage::getContextValue('entity')
can return NULL without throwing a ContextException, and this case is not handled.

Steps to reproduce

  1. Enable Layout Builder and Layout Library.
  2. Create a content type with Layout Builder enabled (for example: themag_landing_page).
  3. Create and save a layout library entry for that content type
    (for example: mg_home_page_6 under
    /admin/structure/types/manage/themag_landing_page/layout-library).
  4. Edit the saved layout library item so that there are unsaved changes.
  5. Click the “Discard changes” link, which hits a URL like:

    /admin/structure/types/manage/themag_landing_page/layout-library/mg_home_page_6/discard-changes.

Proposed resolution

Make DiscardLayoutChangesForm::getDescription() robust when the entity context is
missing or NULL. Instead of always calling $entity->label(), first check that
$entity is not NULL, and fall back to the generic message if there is no entity.

public function getDescription() {
  try {
    $entity = $this->sectionStorage->getContextValue('entity');
    if ($entity && $entity->label()) {
      return $this->t('Any unsaved changes to the layout for %label will be discarded. This action cannot be undone.', [
        '%label' => $entity->label(),
      ]);
    }
  }
  catch (\Drupal\Component\Plugin\Exception\ContextException $e) {
    // Fall through to the generic message below.
  }

  // If the entity context is not available or NULL, return a generic message.
  return $this->t('Any unsaved changes to the layout will be discarded. This action cannot be undone.');
}

Remaining tasks

  • Add automated test coverage for the discard changes route when no entity context is available.
  • Verify the fix on a real site using Layout Library and Layout Builder.

User interface changes

  • No visual UI changes. Only prevents a fatal error and ensures the confirmation page always renders with either the entity label or the generic message.

Introduced terminology

  • None.

API changes

  • None. This only hardens an existing form method against NULL context values.

Data model changes

  • None.

Release notes snippet

Fixed a fatal error when discarding unsaved changes from the Layout Builder discard form in some Layout Library scenarios, by handling missing entity context in DiscardLayoutChangesForm::getDescription().

Issue fork drupal-3604077

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

georg3 created an issue. See original summary.

quietone’s picture

Version: 11.3.x-dev » main
Issue tags: -layout_builder, -layout_library, -bug +Needs title update

Hi, Issues for Drupal core should be targeted to the 'main' branch, our primary development branch. Changes are made on the main branch first, and are then back ported as needed according to the Core change policies. The version the problem was discovered on should be stated in the issue summary Problem/Motivation section. Thanks.

Changing tags per Issue tags field and Issue tags -- special tags

danielveza made their first commit to this issue’s fork.

danielveza’s picture

Title: Layout Library “Discard changes” throws fatal error: Call to a member function label() on null in DiscardLayoutChangesForm::getDescription() » DiscardLayoutChangesForm/RevertOverridesForm::getDescription do not handle NULL context values
Status: Active » Needs review
Issue tags: -Needs title update

I was debating if this is an issue with Layout Library or Core, but the report is correct that getContextValue can return NULL. So we should handle that. I've added a fix & test.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Bug Smash Initiative

Since I can't run test-only jobs (super annoying) ran locally

DiscardLayoutChangesFormTest

Error: Call to a member function label() on null
/var/www/html/core/modules/layout_builder/src/Form/DiscardLayoutChangesForm.php:89
/var/www/html/core/modules/layout_builder/tests/src/Unit/DiscardLayoutChangesFormTest.php:56

RevertOverridesFormTest

Failed asserting that two strings are identical.
Expected :'The layout for <em class="placeholder">My Node</em> will be reverted to its default state. All layout modifications and inline blocks will be reset.'
Actual   :'The layout for <em class="placeholder">My Node</em> will be reverted to its default state. All layout modifications and inline blocks wil be reset.'

Change itself looks fine to me, good defensive code.

Agree with putting the fix into core but maybe an investigation ticket into Layout Library? May be a bug over there too triggering this. Haven't looked though.

  • larowlan committed 0b2b2c3b on 11.x
    fix: #3604077 DiscardLayoutChangesForm/RevertOverridesForm::...

  • larowlan committed 6765430c on main
    fix: #3604077 DiscardLayoutChangesForm/RevertOverridesForm::...

larowlan’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed a57059c9dcd to main and 0b2b2c3b843 to 11.x. Thanks!

Didn't backport to 11.4.x because there are string changes

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.