Site Health: count of inactive themes counts default and active themes separately, even when identical

[johnpgreen]

I have nine themes installed in a development environment (the nine Twenty* themes) and Twenty Nineteen is the active theme. The Site Health Status page reports the following:

Your site has 9 installed themes, and they are all up to date.

Your site has 7 inactive themes, other than Twenty Nineteen, your active theme. We recommend removing any unused themes to enhance your site’s security.

It should report that there are 8 inactive themes.

See: ​https://github.com/WordPress/health-check/issues/205

[ianbelanger]

First pass at fixing issue

[ianbelanger]

My approach is a little different than @sudhiryadav. I'll leave it up to everyone to test and choose the best solution.

[mukesh27]

Both patch works fine for me but i think we have to go with @ianbelanger solution/patch as it will remove the code which we do not needed if we set $allowed_theme_count = 1; and it was already define in file so remove condition code is good solution for this ticket

[Clorith]

This is slightly simpler than removing anything.

We need that counter to accommodate default themes existing when a non-default is active, for the inactive themes that "shouldn't" be kept around.

// If there's a default theme installed and not in use, we count that as allowed as well.
if ( $has_default_theme && ! $using_default_theme ) {
        $allowed_theme_count++;
}

This should negate the unwanted behavior in the correct manner I believe, only increasing the counter if we're not actually using the default theme.

[mukesh27]

Updated patch.

[ianbelanger]

I tested the most recent patch and it does, sort of, fix the issue, but with 2 caveats.

When using a child theme where Twenty Nineteen is not the parent, the theme count is correct, but the other than twentynineteen, the default WordPress theme text is missing.

If you use a child theme, with Twenty Nineteen as the parent, the count and text is incorrect as well.

Screenshots coming

[azaozz]

Replying to ianbelanger:

When using a child theme where Twenty Nineteen is not the parent, the theme count is correct, but the other than twentynineteen, the default WordPress theme text is missing.

Hm, may be missing something but the second screenshot you uploaded says just that: keep TwentyNineteen, your current active child theme and it's parent theme:

If you use a child theme, with Twenty Nineteen as the parent, the count and text is incorrect

If I understand this right, we also need to check if an active child theme is using the default theme as parent. Then set $using_default_theme = true; to not count it twice.

Patch coming up.

[azaozz]

In 46900.3.diff: Count the default theme as "in use" when it is the parent of an active child theme. Fixes themes counts and mentioning the default theme twice when suggesting which themes to keep: as parent and as a default.

[ianbelanger]

Replying to azaozz:

When using a child theme where Twenty Nineteen is not the parent, the theme count is correct, but the other than twentynineteen, the default WordPress theme text is missing.

Hm, may be missing something but the second screenshot you uploaded says just that: keep TwentyNineteen, your current active child theme and it's parent theme:

Why I considered the second image to be incorrect is that it is missing the other than twentynineteen, the default WordPress theme text. When you are not using a child theme, or TwentyNineteen as your active theme, the other than twentynineteen, the default WordPress theme text is shown, but when using a child theme, whether it's a child theme of TwentyNineteen or not, that text is not shown.

I will test the current patch and report back.

[azaozz]

Replying to ianbelanger:

Yeah, the wording is a bit different when using a child theme but thinking it is pretty clear that the user needs to keep three themes:

You should keep ...., the WordPress default theme, ...., your current theme, and ...., its parent theme.

Looked at this again and think 46900.3.diff works as expected. Going to commit it and close the ticket. Feel free to reopen (or make another ticket) if there are other edge cases.

[azaozz]

In 45260:

Site Health: Fix count of inactive themes and the recommendation to remove them when the default theme is active or is a parent of the active child theme.

Props sudhiryadav, ianbelanger, mukesh27, azaozz.
Fixes #46900.