From 38dd1c404d6183d9575b71f567b18f7ccd8f38f1 Mon Sep 17 00:00:00 2001 From: catch <6915-catch@users.noreply.drupalcode.org> Date: Mon, 3 Mar 2025 17:45:11 +0000 Subject: [PATCH] Issue #3493671 by godotislate, plopesc, nicxvan: Placing a block containing a form in navigation layout prevents layout builder from saving --- core/.phpstan-baseline.php | 12 --- .../layout_builder.services.yml | 8 -- .../LayoutBuilderHtmlEntityFormController.php | 73 ---------------- .../src/Element/LayoutBuilder.php | 83 +++++++++++++++++++ .../src/Form/DefaultsEntityForm.php | 15 ---- .../Field/FieldWidget/LayoutBuilderWidget.php | 15 ---- .../layout_builder_decoration_test.info.yml | 5 -- ...ayout_builder_decoration_test.services.yml | 6 -- ...DecorationTestHtmlEntityFormController.php | 54 ------------ .../Hook/LayoutBuilderFormBlockTestHooks.php | 24 ++++++ .../src/Functional/LayoutBuilderTest.php | 17 ---- .../navigation/css/base/layout-builder.css | 2 +- .../css/base/layout-builder.pcss.css | 2 +- .../NavigationBlockUiTest.php | 40 ++++++++- 14 files changed, 146 insertions(+), 210 deletions(-) delete mode 100644 core/modules/layout_builder/src/Controller/LayoutBuilderHtmlEntityFormController.php delete mode 100644 core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.info.yml delete mode 100644 core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.services.yml delete mode 100644 core/modules/layout_builder/tests/modules/layout_builder_decoration_test/src/Controller/LayoutBuilderDecorationTestHtmlEntityFormController.php create mode 100644 core/modules/layout_builder/tests/modules/layout_builder_form_block_test/src/Hook/LayoutBuilderFormBlockTestHooks.php diff --git a/core/.phpstan-baseline.php b/core/.phpstan-baseline.php index 6bfcd63daeb0..12721b05d989 100644 --- a/core/.phpstan-baseline.php +++ b/core/.phpstan-baseline.php @@ -22465,12 +22465,6 @@ 'count' => 1, 'path' => __DIR__ . '/modules/layout_builder/src/Form/DefaultsEntityForm.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Method Drupal\\\\layout_builder\\\\Form\\\\DefaultsEntityForm\\:\\:layoutBuilderElementGetKeys\\(\\) has no return type specified\\.$#', - 'identifier' => 'missingType.return', - 'count' => 1, - 'path' => __DIR__ . '/modules/layout_builder/src/Form/DefaultsEntityForm.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Drupal\\\\layout_builder\\\\Form\\\\DefaultsEntityForm\\:\\:redirectOnSubmit\\(\\) has no return type specified\\.$#', 'identifier' => 'missingType.return', @@ -22771,12 +22765,6 @@ 'count' => 1, 'path' => __DIR__ . '/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Method Drupal\\\\layout_builder\\\\Plugin\\\\Field\\\\FieldWidget\\\\LayoutBuilderWidget\\:\\:layoutBuilderElementGetKeys\\(\\) has no return type specified\\.$#', - 'identifier' => 'missingType.return', - 'count' => 1, - 'path' => __DIR__ . '/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Drupal\\\\layout_builder\\\\Plugin\\\\Layout\\\\MultiWidthLayoutBase\\:\\:submitConfigurationForm\\(\\) has no return type specified\\.$#', 'identifier' => 'missingType.return', diff --git a/core/modules/layout_builder/layout_builder.services.yml b/core/modules/layout_builder/layout_builder.services.yml index 65c93652209b..b3cb73316d83 100644 --- a/core/modules/layout_builder/layout_builder.services.yml +++ b/core/modules/layout_builder/layout_builder.services.yml @@ -57,14 +57,6 @@ services: class: Drupal\layout_builder\InlineBlockUsage arguments: ['@database'] Drupal\layout_builder\InlineBlockUsageInterface: '@inline_block.usage' - layout_builder.controller.entity_form: - # Override the entity form controller to handle the entity layout_builder - # operation. - decorates: controller.entity_form - class: Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController - public: false - arguments: ['@layout_builder.controller.entity_form.inner'] - Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController: '@layout_builder.controller.entity_form' layout_builder.element.prepare_layout: class: Drupal\layout_builder\EventSubscriber\PrepareLayout arguments: ['@layout_builder.tempstore_repository', '@messenger'] diff --git a/core/modules/layout_builder/src/Controller/LayoutBuilderHtmlEntityFormController.php b/core/modules/layout_builder/src/Controller/LayoutBuilderHtmlEntityFormController.php deleted file mode 100644 index 5a3ea7d712a4..000000000000 --- a/core/modules/layout_builder/src/Controller/LayoutBuilderHtmlEntityFormController.php +++ /dev/null @@ -1,73 +0,0 @@ -<?php - -namespace Drupal\layout_builder\Controller; - -use Drupal\Component\Utility\NestedArray; -use Drupal\Core\Controller\FormController; -use Drupal\Core\DependencyInjection\DependencySerializationTrait; -use Drupal\Core\Routing\RouteMatchInterface; -use Symfony\Component\HttpFoundation\Request; - -/** - * Overrides the entity form controller service for layout builder operations. - */ -class LayoutBuilderHtmlEntityFormController extends FormController { - - use DependencySerializationTrait; - - /** - * The entity form controller being decorated. - * - * @var \Drupal\Core\Controller\FormController - */ - protected $entityFormController; - - /** - * Constructs a LayoutBuilderHtmlEntityFormController object. - * - * @param \Drupal\Core\Controller\FormController $entity_form_controller - * The entity form controller being decorated. - */ - public function __construct(FormController $entity_form_controller) { - $this->entityFormController = $entity_form_controller; - } - - /** - * {@inheritdoc} - */ - public function getContentResult(Request $request, RouteMatchInterface $route_match) { - $form = $this->entityFormController->getContentResult($request, $route_match); - - // If the form render element has a #layout_builder_element_keys property, - // first set the form element as a child of the root render array. Use the - // keys to get the layout builder element from the form render array and - // copy it to a separate child element of the root element to prevent any - // forms within the layout builder element from being nested. - if (isset($form['#layout_builder_element_keys'])) { - $build['form'] = &$form; - $layout_builder_element = &NestedArray::getValue($form, $form['#layout_builder_element_keys']); - $build['layout_builder'] = $layout_builder_element; - // Remove the layout builder element within the form. - $layout_builder_element = []; - return $build; - } - - // If no #layout_builder_element_keys property, return form as is. - return $form; - } - - /** - * {@inheritdoc} - */ - protected function getFormArgument(RouteMatchInterface $route_match) { - return $this->entityFormController->getFormArgument($route_match); - } - - /** - * {@inheritdoc} - */ - protected function getFormObject(RouteMatchInterface $route_match, $form_arg) { - return $this->entityFormController->getFormObject($route_match, $form_arg); - } - -} diff --git a/core/modules/layout_builder/src/Element/LayoutBuilder.php b/core/modules/layout_builder/src/Element/LayoutBuilder.php index 2676d5a0dba6..e17e36ab4fb9 100644 --- a/core/modules/layout_builder/src/Element/LayoutBuilder.php +++ b/core/modules/layout_builder/src/Element/LayoutBuilder.php @@ -2,12 +2,16 @@ namespace Drupal\layout_builder\Element; +use Drupal\Component\Utility\Html; +use Drupal\Component\Utility\NestedArray; use Drupal\Core\Ajax\AjaxHelperTrait; +use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Plugin\PluginFormInterface; use Drupal\Core\Render\Attribute\RenderElement; use Drupal\Core\Render\Element; use Drupal\Core\Render\Element\RenderElementBase; +use Drupal\Core\Security\Attribute\TrustedCallback; use Drupal\Core\Url; use Drupal\layout_builder\Context\LayoutBuilderContextTrait; use Drupal\layout_builder\Event\PrepareLayoutEvent; @@ -75,9 +79,87 @@ public function getInfo() { '#pre_render' => [ [$this, 'preRender'], ], + '#process' => [ + [static::class, 'layoutBuilderElementGetKeys'], + ], ]; } + /** + * Form element #process callback. + * + * Save the layout builder element array parents as a property on the top form + * element, so that they can be used to access the element within the form + * render array later. + * + * @param array $element + * The render array for the layout builder element. + * @param \Drupal\Core\Form\FormStateInterface $form_state + * Form state object. + * @param array $form + * The render array for the complete form. + * + * @return array + * The layout builder element render array after processing. + * + * @see \Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController + */ + public static function layoutBuilderElementGetKeys(array $element, FormStateInterface $form_state, array &$form): array { + $form['#layout_builder_element_keys'] = $element['#array_parents']; + $form['#pre_render'][] = [static::class, 'renderLayoutBuilderAfterForm']; + $form['#post_render'][] = [static::class, 'addRenderedLayoutBuilder']; + return $element; + } + + /** + * Render API #pre_render callback for form containing layout builder element. + * + * Because the layout builder element can contain components with forms, it + * needs to exist outside forms within the DOM, to avoid nested form tags. + * The layout builder element is rendered to markup here and saved, and later + * the saved markup will be appended after the form markup. + * + * @param array $form + * The rendered form. + * + * @return array + * + * @see ::addRenderedLayoutBuilder() + */ + #[TrustedCallback] + public static function renderLayoutBuilderAfterForm(array $form): array { + if (isset($form['#layout_builder_element_keys'])) { + $layout_builder_element = &NestedArray::getValue($form, $form['#layout_builder_element_keys']); + // Save the rendered layout builder HTML to a non-rendering child key. + // Since this method is a pre_render callback, it is assumed that it is + // called while rendering with an active render context, so that the + // cache metadata and attachments bubble correctly. + $form['#layout_builder_markup'] = \Drupal::service('renderer')->render($layout_builder_element); + // Remove the layout builder child element within form array. + $layout_builder_element = []; + } + return $form; + } + + /** + * Render API #post_render callback that adds layout builder markup to form. + * + * @param string $html + * The rendered form. + * @param array $form + * The form render array. + * + * @return string + */ + #[TrustedCallback] + public static function addRenderedLayoutBuilder(string $html, array $form): string { + if (isset($form['#layout_builder_markup'])) { + $html .= $form['#layout_builder_markup']; + } + + return $html; + } + /** * Pre-render callback: Renders the Layout Builder UI. */ @@ -122,6 +204,7 @@ protected function layout(SectionStorageInterface $section_storage) { $output['#type'] = 'container'; $output['#attributes']['id'] = 'layout-builder'; $output['#attributes']['class'][] = 'layout-builder'; + $output['#attributes']['class'][] = Html::getClass('layout-builder--' . $section_storage->getPluginId()); // Mark this UI as uncacheable. $output['#cache']['max-age'] = 0; return $output; diff --git a/core/modules/layout_builder/src/Form/DefaultsEntityForm.php b/core/modules/layout_builder/src/Form/DefaultsEntityForm.php index a23cc6097cc9..a793e1734dec 100644 --- a/core/modules/layout_builder/src/Form/DefaultsEntityForm.php +++ b/core/modules/layout_builder/src/Form/DefaultsEntityForm.php @@ -74,7 +74,6 @@ public function buildForm(array $form, FormStateInterface $form_state, ?SectionS $form['layout_builder'] = [ '#type' => 'layout_builder', '#section_storage' => $section_storage, - '#process' => [[static::class, 'layoutBuilderElementGetKeys']], ]; $form['layout_builder_message'] = $this->buildMessage($section_storage->getContextValue('display')); @@ -82,20 +81,6 @@ public function buildForm(array $form, FormStateInterface $form_state, ?SectionS return parent::buildForm($form, $form_state); } - /** - * Form element #process callback. - * - * Save the layout builder element array parents as a property on the top form - * element so that they can be used to access the element within the whole - * render array later. - * - * @see \Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController - */ - public static function layoutBuilderElementGetKeys(array $element, FormStateInterface $form_state, &$form) { - $form['#layout_builder_element_keys'] = $element['#array_parents']; - return $element; - } - /** * Renders a message to display at the top of the layout builder. * diff --git a/core/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php b/core/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php index a6182a2832ec..85da990a3ac0 100644 --- a/core/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php +++ b/core/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php @@ -31,21 +31,6 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen '#type' => 'layout_builder', '#section_storage' => $this->getSectionStorage($form_state), ]; - $element['#process'][] = [static::class, 'layoutBuilderElementGetKeys']; - return $element; - } - - /** - * Form element #process callback. - * - * Save the layout builder element array parents as a property on the top form - * element so that they can be used to access the element within the whole - * render array later. - * - * @see \Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController - */ - public static function layoutBuilderElementGetKeys(array $element, FormStateInterface $form_state, &$form) { - $form['#layout_builder_element_keys'] = $element['#array_parents']; return $element; } diff --git a/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.info.yml b/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.info.yml deleted file mode 100644 index 8d373e72e29e..000000000000 --- a/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.info.yml +++ /dev/null @@ -1,5 +0,0 @@ -name: 'Layout Builder Decoration test' -type: module -description: 'Support module for testing layout building.' -package: Testing -version: VERSION diff --git a/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.services.yml b/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.services.yml deleted file mode 100644 index 3d47731f441c..000000000000 --- a/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/layout_builder_decoration_test.services.yml +++ /dev/null @@ -1,6 +0,0 @@ -services: - layout_builder_decoration_test.controller.entity_form: - decorates: controller.entity_form - class: Drupal\layout_builder_decoration_test\Controller\LayoutBuilderDecorationTestHtmlEntityFormController - public: false - arguments: ['@layout_builder_decoration_test.controller.entity_form.inner'] diff --git a/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/src/Controller/LayoutBuilderDecorationTestHtmlEntityFormController.php b/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/src/Controller/LayoutBuilderDecorationTestHtmlEntityFormController.php deleted file mode 100644 index 735cb8dd5b71..000000000000 --- a/core/modules/layout_builder/tests/modules/layout_builder_decoration_test/src/Controller/LayoutBuilderDecorationTestHtmlEntityFormController.php +++ /dev/null @@ -1,54 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\layout_builder_decoration_test\Controller; - -use Drupal\Core\Controller\FormController; -use Drupal\Core\Routing\RouteMatchInterface; -use Symfony\Component\HttpFoundation\Request; - -/** - * Overrides the entity form controller service for layout builder decoration test. - */ -class LayoutBuilderDecorationTestHtmlEntityFormController extends FormController { - - /** - * The entity form controller being decorated. - * - * @var \Drupal\Core\Controller\FormController - */ - protected $entityFormController; - - /** - * Constructs a LayoutBuilderDecorationTestHtmlEntityFormController object. - * - * @param \Drupal\Core\Controller\FormController $entity_form_controller - * The entity form controller being decorated. - */ - public function __construct(FormController $entity_form_controller) { - $this->entityFormController = $entity_form_controller; - } - - /** - * {@inheritdoc} - */ - public function getContentResult(Request $request, RouteMatchInterface $route_match) { - return $this->entityFormController->getContentResult($request, $route_match); - } - - /** - * {@inheritdoc} - */ - protected function getFormArgument(RouteMatchInterface $route_match) { - return $this->entityFormController->getFormArgument($route_match); - } - - /** - * {@inheritdoc} - */ - protected function getFormObject(RouteMatchInterface $route_match, $form_arg) { - return $this->entityFormController->getFormObject($route_match, $form_arg); - } - -} diff --git a/core/modules/layout_builder/tests/modules/layout_builder_form_block_test/src/Hook/LayoutBuilderFormBlockTestHooks.php b/core/modules/layout_builder/tests/modules/layout_builder_form_block_test/src/Hook/LayoutBuilderFormBlockTestHooks.php new file mode 100644 index 000000000000..740b96c5e526 --- /dev/null +++ b/core/modules/layout_builder/tests/modules/layout_builder_form_block_test/src/Hook/LayoutBuilderFormBlockTestHooks.php @@ -0,0 +1,24 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\layout_builder_form_block_test\Hook; + +use Drupal\Core\Hook\Attribute\Hook; + +/** + * Hooks for layout_builder_form_block_test. + */ +class LayoutBuilderFormBlockTestHooks { + + /** + * Implements hook_block_alter(). + */ + #[Hook('block_alter')] + public function blockAlter(&$definitions): void { + // Allow a test block containing a form to be placed in navigation via + // layout builder. + $definitions['layout_builder_form_block_test_form_api_form_block']['allow_in_navigation'] = TRUE; + } + +} diff --git a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php index d995dbb714e5..620e2fe4374d 100644 --- a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php +++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php @@ -298,23 +298,6 @@ public function testLayoutBuilderUi(): void { $this->assertSame($expected_labels, $labels); } - /** - * Test decorating controller.entity_form while layout_builder is installed. - */ - public function testHtmlEntityFormControllerDecoration(): void { - $assert_session = $this->assertSession(); - - $this->drupalLogin($this->drupalCreateUser([ - 'configure any layout', - 'administer node display', - ])); - - // Install module that decorates controller.entity_form. - \Drupal::service('module_installer')->install(['layout_builder_decoration_test']); - $this->drupalGet('admin/structure/types/manage/bundle_with_section_field/display/default'); - $assert_session->pageTextContains('Manage Display'); - } - /** * Tests that layout builder checks entity view access. */ diff --git a/core/modules/navigation/css/base/layout-builder.css b/core/modules/navigation/css/base/layout-builder.css index 9d79be57072d..a050f531e05a 100644 --- a/core/modules/navigation/css/base/layout-builder.css +++ b/core/modules/navigation/css/base/layout-builder.css @@ -9,7 +9,7 @@ padding: 0; } -#navigation-layout .layout-builder { +.layout-builder.layout-builder--navigation { border: none; } diff --git a/core/modules/navigation/css/base/layout-builder.pcss.css b/core/modules/navigation/css/base/layout-builder.pcss.css index 4cd61aca79ea..a315dbfc7892 100644 --- a/core/modules/navigation/css/base/layout-builder.pcss.css +++ b/core/modules/navigation/css/base/layout-builder.pcss.css @@ -2,7 +2,7 @@ padding: 0; } -#navigation-layout .layout-builder { +.layout-builder.layout-builder--navigation { border: none; } diff --git a/core/modules/navigation/tests/src/FunctionalJavascript/NavigationBlockUiTest.php b/core/modules/navigation/tests/src/FunctionalJavascript/NavigationBlockUiTest.php index 97628a0bd60d..39ae6026cfb7 100644 --- a/core/modules/navigation/tests/src/FunctionalJavascript/NavigationBlockUiTest.php +++ b/core/modules/navigation/tests/src/FunctionalJavascript/NavigationBlockUiTest.php @@ -32,6 +32,7 @@ class NavigationBlockUiTest extends WebDriverTestBase { 'block_content', 'layout_builder', 'layout_test', + 'layout_builder_form_block_test', 'node', 'field_ui', 'shortcut', @@ -70,6 +71,39 @@ protected function setUp(): void { ]); } + /** + * Tests navigation block admin page exists and functions correctly. + */ + public function testNavigationBlockAdminUiPageNestedForm(): void { + $layout_url = '/admin/config/user-interface/navigation-block'; + $this->drupalLogin($this->adminUser); + + // Edit the layout and add a block that contains a form. + $this->drupalGet($layout_url); + $this->openAddBlockForm('Layout Builder form block test form api form block'); + $this->getSession()->getPage()->checkField('settings[label_display]'); + + // Save the new block, and ensure it is displayed on the page. + $this->getSession()->getPage()->pressButton('Add block'); + $this->assertSession()->assertWaitOnAjaxRequest(); + $this->assertSession()->assertNoElementAfterWait('css', '#drupal-off-canvas'); + $this->assertSession()->addressEquals($layout_url); + $this->assertSession()->pageTextContains('Layout Builder form block test form api form block'); + $this->getSession()->getPage()->pressButton('Save'); + $unexpected_save_message = 'You have unsaved changes'; + $expected_save_message = 'Saved navigation blocks'; + $this->assertSession()->statusMessageNotContains($unexpected_save_message); + $this->assertSession()->statusMessageContains($expected_save_message); + + // Try to save the layout again and confirm it can save because there are no + // nested form tags. + $this->drupalGet($layout_url); + $this->getSession()->getPage()->checkField('toggle_content_preview'); + $this->getSession()->getPage()->pressButton('Save'); + $this->assertSession()->statusMessageNotContains($unexpected_save_message); + $this->assertSession()->statusMessageContains($expected_save_message); + } + /** * Tests navigation block admin page exists and functions correctly. */ @@ -102,7 +136,7 @@ public function testNavigationBlockAdminUiPage(): void { // Remove the shortcut block. $this->assertSession()->pageTextContains('Shortcuts'); - $this->clickContextualLink('form .block-navigation-shortcuts', 'Remove block'); + $this->clickContextualLink('.layout-builder .block-navigation-shortcuts', 'Remove block'); $this->assertOffCanvasFormAfterWait('layout_builder_remove_block'); $this->assertSession()->pageTextContains('Are you sure you want to remove the Shortcuts block?'); $this->assertSession()->pageTextContains('This action cannot be undone.'); @@ -110,7 +144,7 @@ public function testNavigationBlockAdminUiPage(): void { $this->assertSession()->assertWaitOnAjaxRequest(); $this->assertSession()->assertNoElementAfterWait('css', '#drupal-off-canvas'); - $this->assertSession()->elementNotExists('css', 'form .block-navigation-shortcuts'); + $this->assertSession()->elementNotExists('css', '.layout-builder .block-navigation-shortcuts'); // Add a new block. $this->getSession()->getPage()->uncheckField('toggle_content_preview'); @@ -143,7 +177,7 @@ public function testNavigationBlockAdminUiPage(): void { // Reconfigure a block and ensure that the layout content is updated. $this->drupalGet($layout_url); - $this->clickContextualLink('form .block-navigation-shortcuts', 'Configure'); + $this->clickContextualLink('.layout-builder .block-navigation-shortcuts', 'Configure'); $this->assertOffCanvasFormAfterWait('layout_builder_update_block'); $page->fillField('settings[label]', 'Newer Shortcuts'); -- GitLab