If a theme does not have both logo and favicon features, then saving the theme settings for that theme will produce the error:

Recoverable fatal error: Argument 1 passed to _file_save_upload_from_form() must be of the type array, null given, called in ...\core\modules\system\src\Form\ThemeSettingsForm.php on line 392 and defined in _file_save_upload_from_form() (line 761 of core\modules\file\file.module).

Example theme.info.yml:

name: Test
type: theme
core: 8.x
features:
  - logo

This is because of lines 383:
$file = _file_save_upload_from_form($form['logo']['settings']['logo_upload'], $form_state, 0);
and 392:
$file = _file_save_upload_from_form($form['favicon']['settings']['favicon_upload'], $form_state, 0);
in ThemeSettingsForm.php not checking the existence of those form elements first.

CommentFileSizeAuthor
#21 interdiff.txt614 byteslauriii
#15 theme-settings-form-validation-2954884-15.patch4.14 KBmsankhala
#4 theme-settings-form-validation-2954884-4.patch1.9 KBmsankhala

Comments

Wongjn created an issue. See original summary.

cilefen’s picture

cilefen commented
Component: forms system » theme system
msankhala’s picture

msankhala commented
Assigned: Unassigned » msankhala
msankhala’s picture

msankhala commented
Assigned: msankhala » Unassigned
Status: Active » Needs review
StatusFileSize
new theme-settings-form-validation-2954884-4.patch1.9 KB

I added an isset check for both logo and favicon in \Drupal\system\Form\ThemeSettingsForm::validateForm. Also removed two unused variable declarations $validators = ['file_validate_is_image' => []]; and $validators = ['file_validate_extensions' => ['ico png gif jpg jpeg apng svg']];

mudassar774’s picture

mudassar774 commented

Hi,
I applied this patch while using core 8.5.0.
No errors after saving theme settings but if i upload logo using file uploader it don't works though if i give path to logo it works.
thanks

msankhala’s picture

msankhala commented

@mudassar774 This patch is for 8.6.x branch I am not sure if this will work well with 8.5.0 drupal version. Can you please provide the error you got while uploading the image?

mudassar774’s picture

mudassar774 commented

In logs I can find this
Notice: Undefined index: base in _color_rewrite_stylesheet() (line 545 of /var/www/site/web/core/modules/color/color.module) #0 /var/www/site/web/core/includes/bootstrap.inc(582): _drupal_error_handler_real(8, 'Undefined index...', '/var/www/site/w...', 545, Array) #1 /var/www/site/web/core/modules/color/color.module(545): _drupal_error_handler(8, 'Undefined index...', '/var/www/site/w...', 545, Array) #2 /var/www/site/web/core/modules/color/color.module(496): _color_rewrite_stylesheet('mobidic', Array, Array, Array, Array) #3 [internal function]: color_scheme_form_submit(Array, Object(Drupal\Core\Form\FormState)) #4 /var/www/site/web/core/lib/Drupal/Core/Form/FormSubmitter.php(111): call_user_func_array('color_scheme_fo...', Array) #5 /var/www/site/web/core/lib/Drupal/Core/Form/FormSubmitter.php(51): Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object(Drupal\Core\Form\FormState)) #6 /var/www/site/web/core/lib/Drupal/Core/Form/FormBuilder.php(585): Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object(Drupal\Core\Form\FormState)) #7 /var/www/site/web/core/lib/Drupal/Core/Form/FormBuilder.php(314): Drupal\Core\Form\FormBuilder->processForm('system_theme_se...', Array, Object(Drupal\Core\Form\FormState)) #8 /var/www/site/web/core/lib/Drupal/Core/Controller/FormController.php(74): Drupal\Core\Form\FormBuilder->buildForm('system_theme_se...', Object(Drupal\Core\Form\FormState)) #9 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch)) #10 /var/www/site/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array) #11 /var/www/site/web/core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() #12 /var/www/site/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) #13 /var/www/site/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) #14 /var/www/site/vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() #15 /var/www/site/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) #16 /var/www/site/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #17 /var/www/site/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #18 /var/www/site/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #19 /var/www/site/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true) #20 /var/www/site/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #21 /var/www/site/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(50): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #22 /var/www/site/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #23 /var/www/site/web/core/lib/Drupal/Core/DrupalKernel.php(657): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #24 /var/www/site/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) #25 {main}.

wongjn’s picture

wongjn commented

Patch works for me on 8.5.0 – seems to be an unrelated issue with the color module.

vishalkhode’s picture

vishalkhode commented

Patch works for me.

vishalkhode’s picture

vishalkhode commented
Status: Needs review » Reviewed & tested by the community
pifagor’s picture

pifagor commented

+1

andrew-as’s picture

andrew-as commented

+1

krishnarp’s picture

krishnarp commented

Patch works for me.
Thanks

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

As this is a bug, it would be great to have some test coverage for this.

msankhala’s picture

msankhala commented
Status: Needs work » Needs review
StatusFileSize
new theme-settings-form-validation-2954884-15.patch4.14 KB

Here is updated patch with test cases. You can run this single test case in your local by following these steps:

  1. Install simpletest module.
  2. Run the below command.
/usr/bin/php /path/to/core/scripts/run-tests.sh  --url http://yourlocaldrupal.loc:port --dburl mysql://username:password@localhost/dbname --concurrency 1 --color --verbose --class "\Drupal\system\Tests\System\ThemeTest::testThemeSettingsNoLogoNoFavicon"

replace the --url and --dburl values as appropriate.

msankhala’s picture

msankhala commented
Issue tags: -Needs tests
surbz’s picture

surbz commented

Thanks @msankhala for this awesome patch.I can confirm that the patch applies cleanly on branch 8.6.x
Also, marking heme-settings-form-validation-2954884-4.patch as hidden in favor of theme-settings-form-validation-2954884-15.patch to avoid any confusions.

surbz’s picture

surbz commented
Status: Needs review » Reviewed & tested by the community

  • lauriii committed 0db1c04 on 8.6.x 
    Issue #2954884 by msankhala: Cannot save theme settings form for themes...

  • lauriii committed 8e025fd on 8.5.x 
    Issue #2954884 by msankhala: Cannot save theme settings form for themes...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new interdiff.txt614 bytes

Thanks for adding the test coverage. Looks good! I removed one unnecessary comment from the tests on the commit and attached interdiff for that.

Committed 0db1c04 and pushed to 8.6.x. I also cherry-picked 8e025fd and pushed to 8.5.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.