Bug #13657
closedActivating commentpress-modern theme can cause fatal errors if commentpress-core plugin not active
0%
Description
See #13656. I haven't taken any steps to confirm what's going on here. It looks like we have a separate wp-content/themes/commentpress-modern directory, added in #12573. But the commentpress-modern theme is also shipped inside of the commentpress-core plugin, which uses register_theme_directory()
to tell WP about it. The latter solution means that the situation should never arise where the plugin is inactive but the theme is active. But our separate copy of the theme is probably overriding the call to register_theme_directory()
Ray, could you please look at what's happening here? I think we want to remove wp-content/themes/commentpress-modern, but I'm not sure if sites currently using commentpress-modern will properly switch over to the bundled version of the theme, or whether we need some sort of migration tool. (Also, there's the further question of what happens when you are using a plugin-bundled theme, via register_theme_directory()
, but the plugin is then deactivated; I assume that this is not what happened in the case of #13656, and I assume it means that WP will just fall back on the default theme, but this could use a check.)
Related issues
Updated by Boone Gorges about 4 years ago
- Related to Bug #13656: site down added
Updated by Boone Gorges about 4 years ago
- Related to Bug #12573: CommentPress Core Issues added
Updated by Boone Gorges about 4 years ago
- Related to Bug #13715: https://ulysses.commons.gc.cuny.edu down added
Updated by Raymond Hoh about 4 years ago
The likely scenario here is a site admin was browsing through the "Appearance > Themes" list and the admin activated one of the CommentPress themes without having the CommentPress Core plugin activated.
I think we should do the following:- Network-disable the
commentpress-default
andcommentpress-modern
themes so these themes are no longer allowed to be shown in the "Appearance > Themes" screen since they only work with thecommentpress-core
plugin. (I've just done this, so no need to do it.) - Remove the
commentpress-theme
andcommentpress-modern
themes from/wp-content/themes/
. WordPress only stores the theme slug in the database, not the absolute path to the theme directory, so we should be safe. (I haven't done this yet. Wanted to get some confirmation before doing this.)
Sites that have either commentpress-theme
or commentpress-modern
should have the commentpress-core
plugin already activated. It's not possible to activate just those themes without receiving a fatal error as evidenced in the last few tickets (#13715, #13656).
To be sure, I ran a WP-CLI command to see where both themes are active:
Sites where commentpress-modern is active: +---------+---------------------------------------------+ | blog_id | url | +---------+---------------------------------------------+ | 1603 | https://disswrkshpf13.commons.gc.cuny.edu/ | | 1682 | https://cunyfemtech.commons.gc.cuny.edu/ | | 1800 | https://sicelidas.commons.gc.cuny.edu/ | | 1921 | https://jk82.commons.gc.cuny.edu/ | | 2241 | https://techlitfrench.commons.gc.cuny.edu/ | | 2273 | https://jitpcomments.commons.gc.cuny.edu/ | | 2309 | https://fragments.commons.gc.cuny.edu/ | | 2314 | https://techniques.commons.gc.cuny.edu/ | | 2358 | https://wgs10016.commons.gc.cuny.edu/ | | 2748 | https://malswrites.commons.gc.cuny.edu/ | | 3944 | https://ivanhoetest.commons.gc.cuny.edu/ | | 4004 | https://tristengoodwin.commons.gc.cuny.edu/ | | 4332 | https://stefanomorello.commons.gc.cuny.edu/ | +---------+---------------------------------------------+
Sites where commentpress-theme is active: +---------+--------------------------------------------------+ | blog_id | url | +---------+--------------------------------------------------+ | 1063 | https://promockup2.commons.gc.cuny.edu/ | | 1271 | https://deathofthetermpaper.commons.gc.cuny.edu/ | | 1313 | https://amstprosemsp13.commons.gc.cuny.edu/ | | 1394 | https://thesocialpaper.commons.gc.cuny.edu/ | | 1491 | https://librarians.commons.gc.cuny.edu/ | | 1506 | https://eshtestcac.commons.gc.cuny.edu/ | +---------+--------------------------------------------------+
All sites have the commentpress-core
plugin activated. (eshtestcac.commons.gc.cuny.edu
has another unrelated error, but that site is marked as deleted so I'm not going to spend time debugging that site.)
One other thing I encountered is if a site has the commentpress-core
plugin activated, the other bundled themes for CommentPress (commentpress-theme
and commentpress-flat
) are not visible under the "Appearance > Themes" screen. I've just added a commit that will allow these themes to be activated if the commentpress-core
plugin is active: https://github.com/cuny-academic-commons/cac/commit/1e1ef9c7da7f13b6e75574d6261280434bf018c3. This should properly solve #12573. I've also reported this issue to Christian (who maintains CommentPress) here.
Updated by Boone Gorges about 4 years ago
Network-disable the commentpress-default and commentpress-modern themes so these themes are no longer allowed to be shown in the "Appearance > Themes" screen since they only work with the commentpress-core plugin. (I've just done this, so no need to do it.)
This sounds good to me.
Remove the commentpress-theme and commentpress-modern themes from /wp-content/themes/. WordPress only stores the theme slug in the database, not the absolute path to the theme directory, so we should be safe. (I haven't done this yet. Wanted to get some confirmation before doing this.)
This seems right to me too. Please go ahead with this step.
Thanks for identifying that related upstream issue!
Updated by Raymond Hoh about 4 years ago
- Status changed from New to Staged for Production Release
This seems right to me too. Please go ahead with this step.
I've removed the commentpress themes in the following commit: https://github.com/cuny-academic-commons/cac/commit/5179c959f8876031eaac5e15f2ab32c4909ee598.
Updated by Matt Gold about 4 years ago
Hi Boone -- can you clarify whether this means that no one will be able to use CommentPress going forward?
Updated by Boone Gorges about 4 years ago
- Category name deleted (
WordPress Themes)
Thanks, Ray!
Hi Boone -- can you clarify whether this means that no one will be able to use CommentPress going forward?
No, it's unrelated to whether CommentPress can be used. This change means that it will be impossible to activate a CommentPress theme without first having the CommentPress plugin active.
Updated by Matt Gold about 4 years ago
Got it. Thanks, Boone and Ray. Should we document this is some way so that people understand the order of what they have to do? Or can we trigger a warning to people who try to select the CP theme before activating the plugin? Apologies if you've already done this and I missed it
Updated by Boone Gorges about 4 years ago
The changes we've made ensure that you won't even see the CommentPress themes in the list unless you have the CommentPress plugin activated. In other words, we are eliminating the possibility of the bad combination that caused the problems in related tickets. I don't think any further action is needed.
Updated by Matt Gold about 4 years ago
Okay. I do think we should document this, though. I've cc'ed Scott into the thread. Scott, can you please add something about this to our Help section?
Updated by Matt Gold about 4 years ago
Right, but then you won't even know about CommentPress as a possibility unless you see it in the plugin list. My suggestion for Scott to document this was to make sure that users knew about the possibilities CP offers and how to go about implementing it in the right way.
Updated by Boone Gorges almost 4 years ago
- Status changed from Staged for Production Release to Resolved
Updated by scott voth almost 4 years ago
For the record - new documentation is available here - https://help.commons.gc.cuny.edu/commentpress/