Contrib.social

Favicon Disappears in Theme Settings Form When Default, Upload, or Path Not Specified

Open on Drupal.org →
Created on 11 December 2023, about 1 year ago
Updated 12 December 2023, about 1 year ago

Problem/Motivation

When attempting to set a favicon for a theme in the theme settings, an issue arises if neither default_favicon, favicon_upload, nor favicon_path is explicitly set. This results in the favicon being incorrectly set to FALSE, preventing it from being displayed in the settings form.

The problem originates in the theme.inc file, specifically in line 317, where features.favicon is set to FALSE and cannot be reset to TRUE.

// Generate the path to the favicon.
if ($cache[$theme]->get('features.favicon')) {
  // ... (code snippet)
  $cache[$theme]->set('features.favicon', FALSE);
}

Additionally, in ThemeSettingsForm.php, line 258 ensures that the Favicon details are hidden and cannot be restored:

'input[name="toggle_favicon"]' => ['checked' => FALSE],

Steps to reproduce

  • Navigate to the settings page for your theme, e.g., the backend theme Claro.
  • Uncheck the checkbox "Use the favicon supplied by the theme" and leave the two fields that appear empty.
  • Save the form.
  • The form elements for Favicon no longer appear.

Proposed resolution

Add validation to the form fields to ensure that at least one of the settings (default_favicon, favicon_upload, or favicon_path) is provided. This prevents the issue of the favicon being incorrectly set to FALSE and ensures proper functionality.

ThemeSettingsForm.php, l. 453 in validateForm():

// Check that at least one setting for the favicon is provided.
if (empty($form_state->getValue('default_favicon')) && empty($form_state->getValue('favicon_upload')) && empty($form_state->getValue('favicon_path'))) {
  $form_state->setErrorByName({TheRightElement}, $this->t('Please provide at least one of the following: Default Favicon, Favicon Upload, or Favicon Path.'));
}

Remaining tasks

Release notes snippet

🐛 Bug report
Status

Needs work

Version

10.1 ✨

Component
Theme  →

Last updated 2 minutes ago

Created by

🇩🇰Denmark nicklasmf

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

  • Issue created by @nicklasmf
  • First commit to issue fork.
  • Status changed to Needs review about 1 year ago
  • Comment about 1 year ago →
    🇮🇳India viren18febS

    Hi @NicklasMF
    I have added a changes ,please review this patch.
    Thanks

  • Comment about 1 year ago →
    🇮🇳India Sandeep_k New Delhi

    Verified this on Drupal version- 10.1.7-dev and was able to apply the patch Issues-3407927-favicon-disappears-in-theme-fixes.patch successfully with a warning, see attached screenshot.
    After applying the patch, while trying to reverify the issue site broke when we clicked on the theme setting-

    Steps to Reproduce-
    Go to Appearance, and Click on Setting.

  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    🇵🇭Philippines abhaypai

    Thanks @viren18febs for sharing the fix as proposal.

    Few inputs i wanna share here are :

    1. #3 patch applied successfully via composer
    2. Error is appearing in the settings because you haven't defined correct element for form element name
    3. {TheRightElement} is just a placeholder which @NicklasMF has shared.
    4. Please understand the logic of Default Favicon, Favicon Upload, or Favicon Path and then appropriately replace form element with {TheRightElement} in the code

    Sharing screenshot for reference.
    Also changing the status to Needs work and as a bug will need a test case to move the issue forward.

  • Status changed to Needs review about 1 year ago
  • Comment about 1 year ago →
    🇮🇳India viren18febS

    I have update the patch file with element value, please review.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update about 1 year ago
    Custom Commands Failed
  • Comment about 1 year ago →
    🇮🇳India Sandeep_k New Delhi

    Thanks, @viren18febs for sharing the fix.

    Verified and tested patch Issues-3407927-favicon-disappears-in-theme-fixes.patch on the Drupal version- 10.1.8-dev. The patch was applied successfully and looks good to me.

    Testing Steps:

    1. Apply patch Issues-3407927-favicon-disappears-in-theme-fixes.patch
    2. Go to Appearance> Open Setting.
    3. Uncheck the favicon checkbox & click save.

    Testing Results:
    The user is unable to save while the favicon checkbox is unchecked.

    Moving this ticket to RTBC.

  • Status changed to RTBC about 1 year ago
  • Comment about 1 year ago →
    🇮🇳India Sandeep_k New Delhi
  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    cilefen

    Where are the tests that someone asked for? Also, what is the meaning of all these screenshots of code?

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024