fix: normalize favicon SVG sources

Author: Callin Mullaney

.github/scripts/favicon-portability-smoke.php

@@ -161,12 +161,17 @@ function emulsify_favicon_run_sanitizer_matrix(FaviconPackageGenerator $generato
   emulsify_favicon_assert(!str_contains(strtolower((string) $analysis['sanitized_svg']), 'onclick='), 'Inline event handlers should be stripped.');
   emulsify_favicon_assert(!str_contains(strtolower((string) $analysis['sanitized_svg']), 'onload='), 'Root event handlers should be stripped.');
 
-  // Hard rejects protect package generation from non-square or excessive input.
-  emulsify_favicon_assert_invalid(
-    fn() => $generator->validateSourceSvg('<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 64 32"><rect width="64" height="32"/></svg>', FALSE),
-    'square viewBox',
+  $analysis = $generator->validateSourceSvg('<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 64 32"><rect width="64" height="32"/></svg>', FALSE);
+  emulsify_favicon_assert(
+    ($analysis['view_box'] ?? []) === [0.0, -16.0, 64.0, 64.0],
+    'Non-square SVG sources should be centered on a square viewBox.',
+  );
+  emulsify_favicon_assert(
+    str_contains((string) $analysis['sanitized_svg'], 'width="64" height="64"'),
+    'Non-square SVG sources should receive square root dimensions.',
   );
 
+  // Hard rejects protect package generation from excessive input.
   $oversized_svg = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 64 64"><desc>' . str_repeat('x', FaviconPackageGenerator::MAX_FILE_SIZE) . '</desc></svg>';
   emulsify_favicon_assert_invalid(
     fn() => $generator->validateSourceSvg($oversized_svg, FALSE),

docs/favicon-generation.md

@@ -9,7 +9,7 @@ Generated PNG and ICO assets require both PHP extensions:
 - GD
 - Imagick
 
-The uploaded source must be an SVG file with a square `viewBox`. If root `width` and `height` values are present, they must also describe a square. The generator accepts embedded raster image data inside the SVG, but the theme settings UI warns that fully vector sources usually scale more cleanly.
+The uploaded source must be an SVG file with a `viewBox`. Non-square sources are centered on a square canvas before package generation so the original aspect ratio is preserved across browser, iOS, and Android assets. The generator accepts embedded raster image data inside the SVG, but the theme settings UI warns that fully vector sources usually scale more cleanly.
 
 Source uploads are limited to 5 MB. The sanitized portable SVG copy is stored in theme config for portability; copies larger than 256 KB are allowed but flagged as review noise because they make config exports harder to inspect.
 

src/Favicon/FaviconHeadBuilder.php

@@ -41,26 +41,26 @@ public function apply(array &$attachments, array $settings): void {
       'rel' => 'icon',
       'href' => $this->fileUrlGenerator->generateString($package_path . '/favicon.ico'),
       'sizes' => 'any',
-    ], 'emulsify_favicon_ico',
+    ], FALSE,
     ];
 
     $attachments['#attached']['html_head_link'][] = [[
       'rel' => 'icon',
       'type' => 'image/svg+xml',
       'href' => $this->fileUrlGenerator->generateString($package_path . '/favicon.svg'),
-    ], 'emulsify_favicon_svg',
+    ], FALSE,
     ];
 
     $attachments['#attached']['html_head_link'][] = [[
       'rel' => 'apple-touch-icon',
       'href' => $this->fileUrlGenerator->generateString($package_path . '/apple-touch-icon.png'),
-    ], 'emulsify_favicon_ios',
+    ], FALSE,
     ];
 
     $attachments['#attached']['html_head_link'][] = [[
       'rel' => 'manifest',
       'href' => $this->fileUrlGenerator->generateString($package_path . '/site.webmanifest'),
-    ], 'emulsify_favicon_manifest',
+    ], FALSE,
     ];
 
     $attachments['#attached']['html_head'][] = [[

src/Favicon/FaviconPackageGenerator.php

@@ -620,18 +620,21 @@ private function inspectSvgMarkup(string $source_data): array {
     if ($view_box === NULL) {
       throw new \InvalidArgumentException('The uploaded SVG must define a viewBox.');
     }
-    if (!$this->isSquare($view_box[2], $view_box[3])) {
-      throw new \InvalidArgumentException('The uploaded SVG must use a square viewBox.');
-    }
 
     $declared_dimensions = $this->extractDeclaredDimensions($root);
-    if ($declared_dimensions['width'] !== NULL && $declared_dimensions['height'] !== NULL && !$this->isSquare($declared_dimensions['width'], $declared_dimensions['height'])) {
-      throw new \InvalidArgumentException('The uploaded SVG must declare square width and height values.');
-    }
+    $view_box_was_normalized = !$this->isSquare($view_box[2], $view_box[3]);
+    $dimensions_were_normalized = $declared_dimensions['width'] !== NULL
+      && $declared_dimensions['height'] !== NULL
+      && !$this->isSquare($declared_dimensions['width'], $declared_dimensions['height']);
 
     $has_embedded_raster_images = $this->documentHasRasterImages($document);
     $uses_transparency = $this->documentUsesTransparency($document);
 
+    if ($view_box_was_normalized || $dimensions_were_normalized) {
+      $view_box = $this->normalizeSvgCanvas($root, $view_box);
+      $declared_dimensions = $this->extractDeclaredDimensions($root);
+    }
+
     $this->stripDisallowedSvgNodes($document);
     $this->stripDisallowedSvgAttributes($document);
 
@@ -641,6 +644,9 @@ private function inspectSvgMarkup(string $source_data): array {
     }
 
     $warnings = [];
+    if ($view_box_was_normalized || $dimensions_were_normalized) {
+      $warnings[] = 'This SVG was centered on a square canvas so generated favicon assets keep the original aspect ratio.';
+    }
     if ($uses_transparency) {
       $warnings[] = 'This SVG appears to use transparency. Check the browser, iOS, and Android previews against their configured backgrounds.';
     }
@@ -659,6 +665,43 @@ private function inspectSvgMarkup(string $source_data): array {
     ];
   }
 
+  /**
+   * Centers a non-square SVG root on a square canvas.
+   *
+   * @param float[] $view_box
+   *   Parsed min-x, min-y, width, and height values.
+   *
+   * @return float[]
+   *   The normalized square viewBox.
+   */
+  private function normalizeSvgCanvas(\DOMElement $root, array $view_box): array {
+    $size = max($view_box[2], $view_box[3]);
+    $normalized_view_box = [
+      $view_box[0] - (($size - $view_box[2]) / 2),
+      $view_box[1] - (($size - $view_box[3]) / 2),
+      $size,
+      $size,
+    ];
+    $formatted_size = $this->formatSvgNumber($size);
+
+    $root->setAttribute('viewBox', implode(' ', array_map(
+      fn (float $value): string => $this->formatSvgNumber($value),
+      $normalized_view_box,
+    )));
+    $root->setAttribute('width', $formatted_size);
+    $root->setAttribute('height', $formatted_size);
+
+    return $normalized_view_box;
+  }
+
+  /**
+   * Formats an SVG number without unnecessary trailing zeroes.
+   */
+  private function formatSvgNumber(float $value): string {
+    $formatted = rtrim(rtrim(sprintf('%.6F', $value), '0'), '.');
+    return $formatted === '-0' ? '0' : $formatted;
+  }
+
   /**
    * Loads an SVG document with safe libxml settings.
    */

src/Favicon/FaviconSettingsForm.php

@@ -252,7 +252,7 @@ public function alter(array &$form, FormStateInterface $form_state): void {
           'extensions' => 'svg',
         ],
       ],
-      '#description' => $this->t('Use a square SVG with a square viewBox. Embedded base64 raster image data is allowed, but it may scale less cleanly than a pure vector source.'),
+      '#description' => $this->t('Use an SVG icon file. Non-square sources are centered on a square canvas. Embedded base64 raster image data is allowed, but it may scale less cleanly than a pure vector source.'),
     ];
     $form['emulsify_favicon']['source']['portable_source_notice'] = [
       '#type' => 'item',
@@ -466,6 +466,10 @@ private function registerCleanValueKeys(FormStateInterface $form_state): void {
    * Validates the generated favicon package settings.
    */
   private function doValidateFaviconSettings(FormStateInterface $form_state): void {
+    if ($this->isFaviconSourceRemoveRequest($form_state)) {
+      return;
+    }
+
     $site_name = $this->getSiteName();
     $settings = FaviconSettings::normalize($form_state->getValues(), $site_name);
     $form_state->setValue('favicon_theme_color', $settings['favicon_android_background_color']);
@@ -497,6 +501,20 @@ private function doValidateFaviconSettings(FormStateInterface $form_state): void
     }
   }
 
+  /**
+   * Returns whether the managed file remove button submitted this rebuild.
+   */
+  private function isFaviconSourceRemoveRequest(FormStateInterface $form_state): bool {
+    $trigger = $form_state->getTriggeringElement();
+    if (!is_array($trigger)) {
+      return FALSE;
+    }
+
+    $array_parents = $trigger['#array_parents'] ?? [];
+    return in_array('favicon_source_fid', $array_parents, TRUE)
+      && end($array_parents) === 'remove_button';
+  }
+
   /**
    * Generates the favicon package when the theme settings form is saved.
    */