Problem/Motivation
In #3073872: Allow for Layout Builder sections to be given administrative labels we added administrative labels, which added a new default configuration 'label' to the LayoutDefault class. However, layouts may choose to use a different class that might not have a label configuration, which results in an notice when trying to remove a section.
Notice: Undefined index: label in Drupal\layout_builder\Form\RemoveSectionForm->getQuestion() (line 28 of /var/www/html/web/core/modules/layout_builder/src/Form/RemoveSectionForm.php)
public function getQuestion() {
$configuration = $this->sectionStorage->getSection($this->delta)->getLayoutSettings();
if ($configuration['label']) {
return $this->t('Are you sure you want to remove @section?', ['@section' => $configuration['label']]);
}
We're already being defensive in other places, such as in Element/LayoutBuilder.php:
$section_label = !empty($layout_settings['label']) ? $layout_settings['label'] : $this->t('Section @section', ['@section' => $delta + 1]);
So we should be here as well!
Steps to reproduce
Proposed resolution
Ensure we check that a label key actually exists in the configuration in RemoveSectionForm.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comments
Comment #2
johnwebdev commentedComment #3
adhershmnair commentedI have reviewed and applied the Patch #2. Patch applied successfully and seems the undefined index notice fixed.
Comment #4
catchShould we add test coverage for this? (not a rhetorical question, if it's a prohibitive amount of work then maybe not).
Could also use a comment explaining why we're being defensive.
Comment #6
raman.b commentedAdding some test coverage to demonstrate the reported bug
Comment #8
johnwebdev commentedComment #9
spokjeRe-uploading
3167901-6.patchso this one gets re-rested every 2 days.Currently the test-only patch
3167901-6-test-only.patchis re-tested every 2 days.Comment #10
catchCommitted/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!
Comment #13
catch