Notice: Undefined index: logo_path in system_theme_settings_validate()

To replicate, edit a theme that does not have a logo feature.

The submission function needs an isset() check on the logo_path & logo_upload values. The same would apply for the favicon_path, replicate using a theme without the favicon feature.

Issues are fixed in head first

I think this should nail it.

Can this not be simplified to

if (isset($form_state['values']['logo_path']) && $form_state['values']['logo_path']) {

to

if (!empty($form_state['values']['logo_path'])) {

And why does the d7 patch have an extra hunk over the d8 one for the logo_upload and favicon_path?

I would look to split them into 3 seperate issues to be honest.

Here is a new D7 patch that incorporates #4's initial work and improves it with comment #5.

Also adds in the missing hunk for the D7 patch.

Confirmed that this does solve the issue for me.

Steps to reproduce:

1. Select a theme with a 'logo' and a 'favicon' feature
2. Edit the appearance for a theme
3. Upload custom logo and favicon

Core behaviour will produce error:

    Notice: Undefined index: logo_path in system_theme_settings_validate() (line 631 of /var/www/site/modules/system/system.admin.inc).
    Notice: Undefined index: logo_upload in system_theme_settings_submit() (line 679 of /var/www/site/modules/system/system.admin.inc).

After the patch is applied, the error is gone.

I am hoping that someone can roll the D8 patch for me ;) Should be very similar - edit, done this below

And here is the D8 patch (hopefully testbox is happier with this one)

Re-rolled patch as I removed 1 line too many, also rolled against latest 8.x-dev

Can anyone help with reviewing these two patched (8.x and 7.x) ?

Thanks

New refactoring based on chx's input attached

Thanks for the patch !Less lines of code, isn't that yummy? And I do not think we need to test for every notice we fix...

Now a patch that does not re-cycle the $key variable ;)

Setting it back to RTBC as per chx comment above

Thanks @wiifm! That looks like the correct fix.

I'm not sure I agree with chx's comment that we don't need tests here. We don't need to test to make sure every array key is set, true, but the fact that the bug did not cause any failures in the test suite indicates that we don't have test coverage for this code.

Also, no RTBCing your own patches please. :) They should always be reviewed first by another person, each time. Thanks!

Marking #1004550: 'Notice: Undefined index' messages if only some features[] are defined in theme .info file as duplicate of this issue.

Regarding testing, does the testing system check for undefined index notices?

#6: 1149900-notices-when-no-logo-d7.patch queued for re-testing.

Duplicate of #1006266: Saving theme-specific theme settings with no logo creates Undefined index error when file module enabled