Cannot save theme settings form for themes without logo or favicon features

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']];

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

@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?

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}.

Patch works for me.

+1

+1

Patch works for me.
Thanks

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

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.

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.

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!