Issue #3436526 by skaught, plopesc, kostyashupenko, m4olivei, quietone, godotislate, catch, ckrina, KeyboardCowboy, nod_, longwave: Adjust custom navigation logo dimensions on upload

core/modules/navigation/config/install/navigation.settings.yml

@@ -1,3 +1,5 @@
 logo_provider: default
-logo_managed: {}
+logo_managed: null
 logo_max_filesize: 1048576
+logo_height: 40
+logo_width: 40

core/modules/navigation/config/schema/navigation.schema.yml

@@ -3,20 +3,45 @@
 navigation.settings:
   type: config_object
   label: 'Navigation settings'
+  constraints:
+    FullyValidatable: ~
   mapping:
     logo_provider:
       type: string
       label: 'Select Navigation logo handling'
       constraints:
-        NotNull: []
+        Choice:
+          - default
+          - hide
+          - custom
     logo_managed:
-      type: sequence
+      type: integer
       label: 'Custom logo'
-      sequence:
+      nullable: true
-        type: integer
+      constraints:
+        Range:
+          min: 0
     logo_max_filesize:
       type: integer
       label: 'Maximum file sizes (bytes)'
+      constraints:
+        NotNull: [ ]
+        Range:
+          min: 0
+    logo_height:
+      type: integer
+      label: 'Logo expected height'
+      constraints:
+        NotNull: [ ]
+        Range:
+          min: 0
+    logo_width:
+      type: integer
+      label: 'Logo expected width'
+      constraints:
+        NotNull: [ ]
+        Range:
+          min: 0
 
 navigation.block_layout:
   type: config_object

core/modules/navigation/navigation.post_update.php

@@ -26,3 +26,16 @@ function navigation_post_update_update_permissions(array &$sandbox) {
     return $needs_save;
   });
 }
+
+/**
+ * Defines the values for the default logo dimensions.
+ */
+function navigation_post_update_set_logo_dimensions_default(array &$sandbox) {
+  $settings = \Drupal::configFactory()->getEditable('navigation.settings');
+  $settings->set('logo_height', 40)
+    ->set('logo_width', 40);
+  if (is_array($settings->get('logo_managed'))) {
+    $settings->set('logo_managed', current($settings->get('logo_managed')));
+  }
+  $settings->save();
+}

core/modules/navigation/src/Form/SettingsForm.php

@@ -7,10 +7,12 @@ namespace Drupal\navigation\Form;
 use Drupal\Component\Utility\Environment;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Config\TypedConfigManagerInterface;
+use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\File\FileSystemInterface;
 use Drupal\Core\File\FileUrlGeneratorInterface;
 use Drupal\Core\Form\ConfigFormBase;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Image\ImageFactory;
 use Drupal\Core\Render\RendererInterface;
 use Drupal\file\Entity\File;
 use Drupal\file\FileUsage\FileUsageInterface;
@@ -24,34 +26,6 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
  */
 final class SettingsForm extends ConfigFormBase {
 
-  /**
-   * The file system service.
-   *
-   * @var \Drupal\Core\File\FileSystemInterface
-   */
-  protected $fileSystem;
-
-  /**
-   * The file usage service.
-   *
-   * @var \Drupal\file\FileUsage\FileUsageInterface
-   */
-  protected $fileUsage;
-
-  /**
-   * The file URL generator.
-   *
-   * @var \Drupal\Core\File\FileUrlGeneratorInterface
-   */
-  protected $fileUrlGenerator;
-
-  /**
-   * Renderer service.
-   *
-   * @var \Drupal\Core\Render\RendererInterface
-   */
-  protected RendererInterface $renderer;
-
   /**
    * Constructs a Navigation SettingsForm object.
    *
@@ -59,28 +33,30 @@ final class SettingsForm extends ConfigFormBase {
    *   The factory for configuration objects.
    * @param \Drupal\Core\Config\TypedConfigManagerInterface $typed_config_manager
    *   The typed config manager.
-   * @param \Drupal\Core\File\FileSystemInterface $file_system
+   * @param \Drupal\Core\File\FileSystemInterface $fileSystem
-   *   File system service.
+   *   The file system.
    * @param \Drupal\Core\File\FileUrlGeneratorInterface $fileUrlGenerator
    *   The file URL generator.
    * @param \Drupal\file\FileUsage\FileUsageInterface $fileUsage
    *   The File Usage service.
    * @param \Drupal\Core\Render\RendererInterface $renderer
-   *   Renderer service.
+   *   The renderer service.
+   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager
+   *   The entity type manager.
+   * @param \Drupal\Core\Image\ImageFactory $imageFactory
+   *   The image factory.
    */
   public function __construct(
     ConfigFactoryInterface $config_factory,
     TypedConfigManagerInterface $typed_config_manager,
-    FileSystemInterface $file_system,
+    protected FileSystemInterface $fileSystem,
-    FileUrlGeneratorInterface $fileUrlGenerator,
+    protected FileUrlGeneratorInterface $fileUrlGenerator,
-    FileUsageInterface $fileUsage,
+    protected FileUsageInterface $fileUsage,
-    RendererInterface $renderer,
+    protected RendererInterface $renderer,
+    protected EntityTypeManagerInterface $entityTypeManager,
+    protected ImageFactory $imageFactory,
   ) {
     parent::__construct($config_factory, $typed_config_manager);
-    $this->fileSystem = $file_system;
-    $this->fileUrlGenerator = $fileUrlGenerator;
-    $this->fileUsage = $fileUsage;
-    $this->renderer = $renderer;
   }
 
   /**
@@ -93,7 +69,9 @@ final class SettingsForm extends ConfigFormBase {
       $container->get('file_system'),
       $container->get('file_url_generator'),
       $container->get('file.usage'),
-      $container->get('renderer')
+      $container->get('renderer'),
+      $container->get('entity_type.manager'),
+      $container->get('image.factory')
     );
   }
 
@@ -130,7 +108,7 @@ final class SettingsForm extends ConfigFormBase {
         NavigationRenderer::LOGO_PROVIDER_HIDE => $this->t('Hide logo'),
         NavigationRenderer::LOGO_PROVIDER_CUSTOM => $this->t('Custom logo'),
       ],
-      '#default_value' => $config->get('logo_provider'),
+      '#config_target' => 'navigation.settings:logo_provider',
     ];
     $form['logo']['image'] = [
       '#type' => 'container',
@@ -141,7 +119,7 @@ final class SettingsForm extends ConfigFormBase {
       ],
     ];
     $allowed = 'png jpg jpeg';
-    $current_logo_managed_fid = $config->get('logo_managed');
+    $current_logo_managed_fid = $config->get('logo_managed') ? [$config->get('logo_managed')] : NULL;
     $max_navigation_allowed = $config->get('logo_max_filesize');
     $max_system_allowed = Environment::getUploadMaxSize();
     $max_allowed = $max_navigation_allowed < $max_system_allowed ? $max_navigation_allowed : $max_system_allowed;
@@ -172,9 +150,13 @@ final class SettingsForm extends ConfigFormBase {
    */
   public function validateForm(array &$form, FormStateInterface $form_state): void {
     $logo_managed = $form_state->getValue('logo_managed');
-    if ($form_state->getValue('logo_provider') === NavigationRenderer::LOGO_PROVIDER_CUSTOM && empty($logo_managed) === TRUE) {
+    if ($form_state->getValue('logo_provider') === NavigationRenderer::LOGO_PROVIDER_CUSTOM && empty($logo_managed)) {
       $form_state->setErrorByName('logo_managed', 'An image file is required with the current logo handling option.');
     }
+
+    // If the upload element is not empty and the image is new, try to adjust
+    // the image dimensions.
+    $this->validateLogoManaged($form, $form_state);
   }
 
   /**
@@ -185,8 +167,7 @@ final class SettingsForm extends ConfigFormBase {
 
     // Get the previous config settings.
     $previous_logo_provider = $config->get('logo_provider');
-    $logo_managed = $config->get('logo_managed');
+    $previous_logo_fid = $config->get('logo_managed');
-    $previous_logo_fid = $logo_managed ? reset($logo_managed) : NULL;
 
     // Get new values from the form.
     $new_logo_provider = $form_state->getValue('logo_provider');
@@ -214,10 +195,82 @@ final class SettingsForm extends ConfigFormBase {
     }
 
     $config
-      ->set('logo_provider', $form_state->getValue('logo_provider'))
+      ->set('logo_managed', $new_logo_fid)
-      ->set('logo_managed', $form_state->getValue('logo_managed'))
       ->save();
     parent::submitForm($form, $form_state);
   }
 
+  /**
+   * Validate the Logo Managed image element.
+   *
+   * @param array $form
+   *   An associative array containing the structure of the form.
+   * @param \Drupal\Core\Form\FormStateInterface $form_state
+   *   The current state of the form.
+   */
+  protected function validateLogoManaged(array $form, FormStateInterface $form_state): void {
+    $logo_managed = $form_state->getValue('logo_managed');
+    $config = $this->config('navigation.settings');
+    if (empty($logo_managed)) {
+      return;
+    }
+
+    $width = $config->get('logo_width');
+    $height = $config->get('logo_height');
+
+    // Skip if the fid has not been modified.
+    $fid = reset($logo_managed);
+    if ($fid == $config->get('logo_managed')) {
+      return;
+    }
+
+    $file = $this->entityTypeManager->getStorage('file')
+      ->load($fid);
+    if ($fid && !$this->adjustLogoDimensions($file)) {
+      $form_state->setErrorByName('logo_managed', $this->t('Image dimensions are bigger than the expected %widthx%height pixels and cannot be used as the navigation logo.',
+        [
+          '%width' => $width,
+          '%height' => $height,
+        ]));
+    }
+  }
+
+  /**
+   * Adjusts the custom logo dimensions according to navigation settings.
+   *
+   * @param \Drupal\file\Entity\File $file
+   *   The file entity that contains the image.
+   *
+   * @return bool
+   *   TRUE if the logo image dimensions are properly adjusted. FALSE otherwise.
+   */
+  protected function adjustLogoDimensions(File $file): bool {
+    $config = $this->config('navigation.settings');
+    $image = $this->imageFactory->get($file->getFileUri());
+    if (!$image->isValid()) {
+      return FALSE;
+    }
+
+    $width = $config->get('logo_width');
+    $height = $config->get('logo_height');
+
+    if ($image->getWidth() <= $width && $image->getHeight() <= $height) {
+      return TRUE;
+    }
+
+    if ($image->scale($width, $height) && $image->save()) {
+      $this->messenger()->addStatus($this->t('The image was resized to fit within the navigation logo expected dimensions of %widthx%height pixels. The new dimensions of the resized image are %new_widthx%new_height pixels.',
+        [
+          '%width' => $width,
+          '%height' => $height,
+          '%new_width' => $image->getWidth(),
+          '%new_height' => $image->getHeight(),
+        ]));
+
+      return TRUE;
+    }
+
+    return FALSE;
+  }
+
 }

core/modules/navigation/src/NavigationRenderer.php

@@ -19,6 +19,7 @@ use Drupal\Core\Plugin\Context\Context;
 use Drupal\Core\Plugin\Context\ContextDefinition;
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\file\Entity\File;
+use Drupal\file\FileInterface;
 use Drupal\layout_builder\SectionStorage\SectionStorageManagerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
 
@@ -143,18 +144,15 @@ final class NavigationRenderer {
     $page_top['navigation'] = $build;
 
     if ($logo_provider === self::LOGO_PROVIDER_CUSTOM) {
-      $logo_managed_fid = $logo_settings->get('logo_managed');
+      $logo_managed = File::load($logo_settings->get('logo_managed'));
-      if (isset($logo_managed_fid[0]) && $logo_managed_fid[0] > 0) {
+      if ($logo_managed instanceof FileInterface) {
-        $logo_managed = File::load($logo_managed_fid[0]);
+        $logo_managed_uri = $logo_managed->getFileUri();
-        if ($logo_managed instanceof File) {
+        $logo_managed_url = $this->fileUrlGenerator->generateAbsoluteString($logo_managed_uri);
-          $logo_managed_uri = $logo_managed->getFileUri();
+        $page_top['navigation'][0]['settings']['logo_path'] = $logo_managed_url;
-          $logo_managed_url = $this->fileUrlGenerator->generateAbsoluteString($logo_managed_uri);
+        $image = $this->imageFactory->get($logo_managed_uri);
-          $page_top['navigation'][0]['settings']['logo_path'] = $logo_managed_url;
+        if ($image->isValid()) {
-          $image = $this->imageFactory->get($logo_managed_uri);
+          $page_top['navigation'][0]['settings']['logo_width'] = $image->getWidth();
-          if ($image->isValid()) {
+          $page_top['navigation'][0]['settings']['logo_height'] = $image->getHeight();
-            $page_top['navigation'][0]['settings']['logo_width'] = $image->getWidth();
-            $page_top['navigation'][0]['settings']['logo_height'] = $image->getHeight();
-          }
         }
       }
     }

core/modules/navigation/tests/src/Functional/NavigationLogoTest.php

@@ -93,7 +93,7 @@ class NavigationLogoTest extends BrowserTestBase {
     // Preset the configuration to verify a custom image is being seen.
     $config = $this->configFactory->getEditable('navigation.settings');
     $config->set('logo_provider', 'custom');
-    $config->set('logo_managed', [$logo_file->id()]);
+    $config->set('logo_managed', $logo_file->id());
     $config->save();
     // Refresh the page to verify custom logo is placed.
     $this->drupalGet('/admin/config/user-interface/navigation/settings');

core/modules/navigation/tests/src/FunctionalJavascript/NavigationSettingsFormTest.php

@@ -0,0 +1,83 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\navigation\FunctionalJavascript;
+
+use Drupal\file\Entity\File;
+use Drupal\FunctionalJavascriptTests\WebDriverTestBase;
+use Drupal\Tests\TestFileCreationTrait;
+
+/**
+ * Tests that theme form settings works correctly.
+ *
+ * @group navigation
+ */
+class NavigationSettingsFormTest extends WebDriverTestBase {
+
+  use TestFileCreationTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['navigation'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    $admin = $this->drupalCreateUser(['administer site configuration']);
+    $this->drupalLogin($admin);
+
+    // Set expected logo dimensions smaller than core provided test images.
+    \Drupal::configFactory()->getEditable('navigation.settings')
+      ->set('logo_height', 10)
+      ->set('logo_width', 10)
+      ->save();
+  }
+
+  /**
+   * Tests that submission handler works correctly.
+   */
+  public function testFormSettingsSubmissionHandler() {
+    $page = $this->getSession()->getPage();
+    $assert_session = $this->assertSession();
+
+    $this->drupalGet("/admin/config/user-interface/navigation/settings");
+
+    // Add a new managed file.
+    $file = current($this->getTestFiles('image'));
+    $image_file_path = \Drupal::service('file_system')->realpath($file->uri);
+    $page->attachFileToField('files[logo_managed]', $image_file_path);
+    $assert_session->waitForButton('logo_managed_remove_button');
+
+    // Assert the new file is uploaded as temporary. This file should not be
+    // saved as permanent if settings are not submitted.
+    $image_field = $this->assertSession()->hiddenFieldExists('logo_managed[fids]');
+    $file = File::load($image_field->getValue());
+    $this->assertFalse($file->isPermanent());
+
+    $page->pressButton('Save configuration');
+    \Drupal::entityTypeManager()->getStorage('file')->resetCache();
+
+    $this->drupalGet("/admin/config/user-interface/navigation/settings");
+
+    // Assert the uploaded file is saved as permanent.
+    $image_field = $this->assertSession()->hiddenFieldExists('logo_managed[fids]');
+    $file = File::load($image_field->getValue());
+    $this->assertTrue($file->isPermanent());
+
+    // Ensure that the image has been resized to fit in the expected container.
+    $image = \Drupal::service('image.factory')->get($file->getFileUri());
+    $this->assertLessThanOrEqual(10, $image->getHeight());
+    $this->assertLessThanOrEqual(10, $image->getWidth());
+  }
+
+}