When a subtheme does not have favicon.ico, then the favicon.ico from the base theme should be used

Agreed. Anyone up for writing the patch for this?

.

I wonna try this. Couple of questions.

1) what is the base theme? Garland?
2) Do i have to make the dirname hardcoded or is their a variable $base_theme or something like that?
3) What is the output of $theme_object->filename, if the filename is "test"

themes/test

or

test

I wonna try this. Couple of questions.

This means I wonna try to patch it XD.
I needed the answers to write a patch

This should be fixed first #761608: Missing theme settings values because list_themes() has inconsistent theme object data so it can be worked through the 'base_themes' data.

Since it has been a couple years since the last update to this ticket, I thought I might take a crack at this.

I downloaded Zen, threw a custom favicon in it, then created my own sub-theme, which was basically just a .info file and a different custom favicon. I went through theme.inc with a debugger and made my changes and then removed the favicon from my subtheme. Once removed, It defaulted to the custom favicon I had set for Zen.

Please have a look and let me know what you think.

Alright, that one failed. I changed it and ran SimpleTest locally and the tests all passed. Hopefully TestBot will agree…

I was told that attention wouldn't be given to this patch unless it was checked and addressed in Drupal 8. After digging around a bit in D8 and learning about how to make subthemes, I discovered that this behavior still exists. The attached patch is the D8 version. I will re-roll the D7 patch so that it more closely resembles its D8 counterpart. Stay tuned.

This is the re-rolled version of the D7 patch. I declared $base_theme and added to the existing inline comments about the favicon to make it resemble the D8 version of the patch. Please disregard the D7 patch in comment in #442814-10: When a subtheme does not have favicon.ico, then the favicon.ico from the base theme should be used.

#13: drupal7-default_favicon-442814-13.patch queued for re-testing.

Haha… Okay I was scratching my head to figure out why TestBot was telling me that the patch wouldn't apply, when I could apply it just fine locally. Turns out… Now that I have changed the version for this ticket to 8.x-dev, it's going to try to apply the patch to that version. I am going to open a separate ticket for the D7 fix, post the patch to it and then append the link to that ticket to this one.

Ignore any other patches but the Drupal 8 version posted in #442814-12: When a subtheme does not have favicon.ico, then the favicon.ico from the base theme should be used. The Drupal 7 version of this patch can be found at #2093955: When a subtheme does not have favicon.ico, then the favicon.ico from the base theme should be used (7.x).

@deetergp the latest patch is what gets considered. having a D7 patch on a D8 issue will cause the bot to keep switching this back to needs work, which will make it hard for someone to know to test and RTBC it.

See also https://drupal.org/node/332678#versions

@deekayen As I understand it, all I need to do in this situation is re-post the D8 patch so that it is the latest/most-recent one in the chain, and then TestBot will review it and leave the ticket in a state of "Needs review". I talked that through with @shrop and he seems to agree that would be the best course of action.

This is getting ridiculous…

Re-rolled, re-tested locally, this should do it, and if it doesn't, I'm taking a break from this for a bit.

Closed #2093955: When a subtheme does not have favicon.ico, then the favicon.ico from the base theme should be used (7.x) as a duplicate. The other issue was created to attempt to backport the changes to 7.x quicker. It contains a 7.x patch that may be used as a starting point for when this issue gets to the backport stage.

Patch no longer applies.

With this change, does that mean a subtheme would have to provide a favicon to override the base theme's favicon? It seems like there should at least be a way for a subtheme to declare it doesn't want to use the base theme's favicon.

patch re-rolled for comment #24

Corrected file name with respective comment.

This seems like a good idea overall. Maybe this should be going through a loop till it finds one incase there are more than one base theme to cover this fully?

Sorry Joel but I don't agree, I think this is a bad idea unless the concerns raised in #23 are solved. Sub themes should definitely not inherit base theme favicon just because.

There is no bug here, this is feature request.

@Jeff Burnz a good point, thanks, a bit of a late night triage didn't look at comments, only code;)
Since it's a feature request I'll be bumping to 8.1.x.

Merge request created based on the patch in #27.

To address #23, perhaps it ought to be possible set a variable in the info file such as favicon: false to indicate that no favicon is to be used.

Think it will need simple test coverage

Re-rolled.

Are there currently tests for favicon.ico? I can't find any that test for a theme's favicon.ico.

We already have ThemeSettingsTest, we should work on extending that vs a new file. Also the test doesn't fully look inline with existing test structure is there an example of that?