Saving theme-specific theme settings with no logo creates Undefined index error when file module enabled

how to reproduce ?? what theme are you using. have you test RC4 & Dev ?

This error still occurs. Use the features[] in your custom theme .info file and do not add the "logo" as a feature. Submitting the theme settings form then prints that error message.

system_theme_settings() only adds the logo-related form fields if logo is an enabled feature. Which is great - but then system_theme_settings_validate() and system_theme_settings_submit both assume that the logo_path field exists.

Line 631 needs to be something like:
if (isset($form_state['values']['logo_path']) && $form_state['values']['logo_path']) {

Line 679
if (isset($values['logo_upload']) && $file = $values['logo_upload']) {

Rolling patch.

patch no longer applies

Rerolling patch

Favicon also needed fixed. New patch and interdiff attached.

I will write the tests next week at core office hours.

Thanks. This is a very annoying bug indeed.

Still needs tests.

I can confirm this bug, see attached images showing error before patch from #9 being applied, and post apply with no error.

With the #9 patch this issue is resolved and works without error.

I used a duplicated (renamed) Stark theme to replicate this, with the theme residing in root/themes.

status prodding

We need tests.

#9: theme_with_no_logo_creates_notice-1006266-9.patch queued for re-testing.

I had begun making tests for this, but couldn't reproduce the undefined index notices with the test. I can reproduce them manually, of course. I'll upload what I have so someone else can continue it, if they see a way to provoke the notices.

The reason I made a new test class for this is that ThemeTest is supposed to test only Theme API functions (and should probably be renamed ThemeApiTest), and there is no other test classes that test the per-theme settings page.

The first patch attached should fail (but does not).
The second patch has the test and the fix combined and should not fail.

#17: theme_with_no_logo_creates_notice-1006266-17-test-only.patch queued for re-testing.

The bot had some issues yesterday.

#17: theme_with_no_logo_creates_notice-1006266-17-combined.patch queued for re-testing.

I can't duplicate this manually in 8.x HEAD. Adding tags to get some more info.

I am trying to recreate the steps for this issue.

According to the original report:

For themes that have the features[] = logo disabled in the theme's info file, the following error shows up:

Since then the info file has become the info.yml file AND the precise syntax for the items written in the info.yml file has changed slightly.

I found the following CHANGE RECORD that shows some of the changes: http://drupal.org/node/1935708

However, the change record does not show exactly how "features" should be written in the new info.yml file. Based upon other items, I'm guessing they would look something like this:

features:
logo: '0'
name: '0'
favicon: '0'

However, I'm not getting the behavior I expect with "features" formatted like this, so I'm not sure if I'm correct. Can anyone help me confirm the correct format for writing features in Drupal 8 info.yml files?

I will continue to experiment.

@stpaultim to help people give a good answer, please make a comment showing the contents of the files, or just attach a tar gz of the theme files you are trying. Also, let us know where you are putting them (in /themes ?)

that way, we can see where you are.

Tried and failed to recreate the problem:

1) Pulled a fresh installation of Drupal with git
2) Set up Drupal 8 with standard installation
3) Made a copy of Stark theme from drupal/core/themes
4) Pasted copy of Stark theme into drupal/themes
5) Made Stark theme the default theme and checked settings page (everything ok)
6) Added following to drupal/themes/stark/stark.info.yml (as described here: http://drupal.org/node/1935708#comment-7224390)

features:
  - logo
  - name
  - slogan
  - node_user_picture
  - comment_user_picture
  - comment_user_verification
  - favicon
  - main_menu
  - secondary_menu

7) Cleared cache and checked stark settings - all nine items still available as options (everything ok)
8) Removed "- favicon" from drupal/themes/stark/stark.info.yml
* Cleared cache and checked stark settings
* "Shortcut Icon" was no longer an option - eight items still available (everything ok)
9) Removed "- slogan" from drupal/themes/stark/stark.info.yml
* Cleared cache and checked stark settings
* "Site Slogan" was no longer an option - seven items still available (everything ok)
10) Removed "- logo" from drupal/themes/stark/stark.info.yml
* Cleared cache and checked stark settings
* "Logo" was no longer an option - six items still available (everything still ok)

I expected it to break at this last step, but it didn't. Either it is now working or I've misunderstood the problem. It seems to me like there may have been significant problems since this was last confirmed as an issue.

those are great step by step instructions!

I think that now, we wait for someone to confirm that *they* can reproduce the problem.

Hmm thinking a little bit more:

I'm wondering if as a 4b you changed the name of the theme to make sure the right stark was being changed.

When you removed the slogan from the info file, did the slogan also disappear from the site?

After step 10, try a step 11 where you save the form (looks like you cleared the cache and reloaded the form).
For example, try changing the site name and then save.

We will need some steps to reproduce that are nice and specific like that for 7.x eventually, which will be a bit different since the info file in 7 does not use yml.

Thanks for the feedback @YesCT

I tried again, this time addressing the issues you mentioned.

Tried and failed to recreate the problem:

1) Pulled a fresh installation of Drupal with git
2) Set up Drupal 8 with standard installation
3) Made a copy of Stark theme from drupal/core/themes
4) Pasted copy of Stark theme into drupal/sites/all/themes
5) Edited the necessary files to make a sub-theme of Stark called "Test"
• changed name of info.yml file to "test.info.yml"
• changed name of folder to "test"
• changed breakpoints.yml file to "test.breakpoints.yml"
• in info.yml file changed "name: Test" & added "base theme: stark"
6) Made test theme the default theme and checked settings page (everything ok)
7) Added a slogan in 'configuration' and uploaded a custom logo under theme appearance.
8) Added following to drupal/sites/all/themes/test/test.info.yml

features:
  - logo
  - name
  - slogan
  - node_user_picture
  - comment_user_picture
  - comment_user_verification
  - favicon
  - main_menu
  - secondary_menu

9) Cleared cache and checked "test" settings - all nine items still available as options (everything ok)
New logo and slogan appear as expected on home page.
10) Removed "- secondary_menu" from drupal/sites/all/themes/test/test.info.yml
* Cleared cache and checked "test" settings
* "Secondary Menu" was no longer an option - eight items still available (everything ok)
11) Removed "- slogan" from drupal/sites/all/themes/test/test.info.yml
* Cleared cache and checked "test" settings
* "Site Slogan" was no longer an option - seven items still available (everything ok)
* Site slogan was no longer visible on home page
12) Removed "- logo" from drupal/sites/all/themes/test/test.info.yml
* Cleared cache and checked "test" settings
* "Logo" was no longer an option - six items still available (everything still ok)
* Logo was no longer visible on home page.
13) Added "- logo" back to drupal/sites/all/themes/test/test.info.yml
* Cleared cache and checked "test" setings
* "Logo" was once again a visible option
* The logo was once again visible on the home page
14) Commented out several features including logo in drupal/sites/all/themes/test/test.info.yml

features:
#  - logo
  - name
  - slogan
#  - node_user_picture
#  - comment_user_picture
#  - comment_user_verification
#  - favicon
  - main_menu
  - secondary_menu

* Cleared cache and checked "test" settings.
* "Logo" was no longer an option
* Logo was no longer visible on the home page

At no point along this entire process did I experience any breaks or error messages.

@YesCT spotted the fact that I was failing to "Save Configuration" after checking the settings as described in #1006266-4: Saving theme-specific theme settings with no logo creates Undefined index error when file module enabled.

Once I realized this, it was easy to create the error messages. I've updated the issues summary to include very detailed instructions on how to recreate this issue. Please, excuse my confusion above.

Thanks for the clear steps @ Stpaultim
i tried to replicate the issue, I followed steps 1-6. when i click on activate and set default the theme, it displays error "The test theme was not found."
is it something wrong with my dev setup ?

I did the same process mentined @stpaultim. I did not encounter any error messages.

Adding detailed steps to recreate the issue

Updated issue summary.

@sidharthap It gave that message because you did not have the Stark theme enabled. I updated the steps to replicate to include that.

Having followed the updated steps I was also able to reproduce the problem.

I think we have confirmed the steps to reproduce, so removing that tag. This still needs tests.

The issue summary could still use some love.

Thank you for the clear steps. This time i replicate the issue. Here is my attachment.

the steps might be able to be simplified. pointing out the step that makes the error.

This error is reproducible by just doing this...

A better test because it is in the correct test and a fix.

My steps to reproduce:

1. Install standard
2. Goto admin/appearance/settings/bartik and press save
3. See PHP notice

Improved this issue title.

Tagging with "rc target" as this user story, changing theme specific settings, is super common and this looks bad. Not critical because site is not broken and nothing bad actually happens.

This still applies and fixes the issue thanks for the test.

Followed the steps to reproduce:

Before

After

Committed/pushed to 8.0.x, thanks!