Make WordPress Core

Opened 14 years ago

Closed 3 years ago

#14955 closed enhancement (maybelater)

Themes should support uninstall.php or uninstall hook

Reported by: wraithkenny's profile WraithKenny Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch
Focuses: Cc:

Description (last modified by scribu)

Related tickets on extending Themes to have Plugin features: #7795 and #14849 but those deal primarily with activation and deactivation centering around 'switch_themes' action.

From looking at /wp-admin/includes/plugin.php, adding support for uninstall is a separate concern (and it doesn't have the issue blocking activation/deactivation).

Should is_uninstallable_plugin, register_uninstall_hook and uninstall_plugin be extended to check theme directory or should versions (is_uninstallable_theme, register_uninstall_theme_hook and uninstall_theme) be added to theme.php?

Advantage of the first method is that the register_uninstall_hook could be reused for Themes where in the second, a new, less attractive name would be needed (register_uninstall_theme_hook?) Also, there's no theme_basename as it was reverted/removed.

Attachments (4)

14955.diff (1013 bytes) - added by greuben 13 years ago.
14955.1.diff (1.2 KB) - added by obenland 11 years ago.
Refreshed patch
14955.2.patch (785 bytes) - added by vetyst 8 years ago.
v4.6 deleted_theme hook
14955.2.diff (1.4 KB) - added by pbiron 5 years ago.
adds delete_theme and deleted_theme actions

Download all attachments as: .zip

Change History (39)

#1 @scribu
14 years ago

We shouldn't attempt to reuse register_*_hook() functions because there's only one theme running at a time, so the underlying logic would be very different.

That's why there's no theme_basename() and why the $file parameter wouldn't be needed.

#2 @scribu
14 years ago

  • Description modified (diff)

#3 @scribu
14 years ago

Forget what I said, as it doesn't apply to the uninstall process.

#4 @scribu
14 years ago

  • Keywords needs-patch 3.2-early added
  • Milestone changed from Awaiting Review to Future Release

#5 @WraithKenny
13 years ago

  • Cc Ken@… added

#6 @scribu
13 years ago

Related: #7795

(duh, it's in the ticket description)

Last edited 13 years ago by scribu (previous) (diff)

#7 @chipbennett
13 years ago

  • Cc chip@… added

So, at Scribu's prompting, I'm going to attempt to create a patch for this. I'm thinking of creating analogous functions in themes.php, unless either of you think I should go the route of extending the existing Plugin hooks?

(Should I assign this to myself? I am still mostly unfamiliar with Trac workflow.)

#8 @scribu
13 years ago

You can assign it to yourself, if you want, but it's not mandatory.

Extending register_uninstall_hook() is not an option.

The available options are:

  • uninstall.php file
  • register_theme_uninstall_hook()

13 years ago

#9 @greuben
13 years ago

  • Keywords 3.3-early has-patch added; needs-patch 3.2-early removed

Instead of register_theme_uninstall_hook() we can use 'uninstall_theme' hook.

14955.diff makes use of uninstall.php and introduces 'uninstall_theme' hook.

#10 @scribu
13 years ago

  • Keywords early added; 3.3-early removed

#11 @DrewAPicture
13 years ago

  • Cc xoodrew@… added

#19505 closed as duplicate.

#12 @oncletom
12 years ago

  • Cc thomas@… added

Any progress on this topic?

#13 @DeanMarkTaylor
11 years ago

  • Cc DeanMarkTaylor added

11 years ago

Refreshed patch

#14 @obenland
11 years ago

  • Keywords early removed
  • Milestone changed from Future Release to 3.7

#16 follow-up: @nacin
11 years ago

This doesn't seem like functionality I'd want themes to have.

#17 @WraithKenny
11 years ago

@nacin Theme's can add options, create tables, and do all sorts of things that plugins can do, but has little opportunity to clean up after themselves. Shouldn't themes clean up like plugins are expected to?

#18 in reply to: ↑ 16 @helen
11 years ago

  • Milestone changed from 3.7 to Awaiting Review

Replying to nacin:

This doesn't seem like functionality I'd want themes to have.

#19 @Apiweb
10 years ago

  • Version 3.0.1 deleted

As the WraithKenny said, has many themes that add tables, alter the structure of WordPress, and with all the powerful API of WordPress they can do a lot more, I believe this would be an interesting option to be implemented.

This ticket was mentioned in Slack in #meta by sterlo. View the logs.

9 years ago

This ticket was mentioned in Slack in #cli by sterlo. View the logs.

9 years ago

#22 @chriscct7
9 years ago

#16401 was marked as a duplicate.

#23 @kjodle
9 years ago

Some themes are doing this already. It would be beneficial to keep the database clean, and to ensure that all themes are doing this in a uniform way. Perhaps as all themes migrate to using the customizer, this can be implemented.

#24 @chriscct7
9 years ago

  • Keywords dev-feedback added

#25 @muxahuk1214
9 years ago

Any progress on this topic? May be you could implement only do_action for now ?

Last edited 9 years ago by muxahuk1214 (previous) (diff)

This ticket was mentioned in Slack in #core by ocean90. View the logs.

8 years ago

#27 @swissspidy
8 years ago

#37951 was marked as a duplicate.

8 years ago

v4.6 deleted_theme hook

#28 @vetyst
8 years ago

  • Severity changed from normal to blocker

I would really want this feature to be implemented aswell, I had a round about way for catching theme deletions before by checking if the script wp-admin/theme.php was used. Unfortuanetly since 4.6 this no longer works as everything goes through ajax.

A simple hook would be sufficient as profided in 14955.2.patch!

#29 @obenland
8 years ago

  • Severity changed from blocker to normal

#30 @swissspidy
8 years ago

#38001 was marked as a duplicate.

#31 @okvee
8 years ago

I have a very simple theme that has no feature, just index.php style.css and screenshot.png.

WordPress have customizer and save settings in the db. (Its name is theme_mods_MYTHEMENAME in the options table.)

When I delete my theme WordPress did not delete that value in the db.

And @nacin said "This doesn't seem like functionality I'd want themes to have."!
I don't think you love junk.

Please have a function to work on delete/uninstall the theme.

#32 @swissspidy
8 years ago

@okvee Theme mods are somewhat different. I understand this ticket to be about helping themes to remove their custom tables etc. on uninstall (which themes really shouldn't use). Theme mods are handled by WordPress and crucial to the overall UX. When changing themes and previewing them in the customizer, I want to have the same modifications (e.g. custom CSS) applied as before. I don't want this data to get lost.

5 years ago

adds delete_theme and deleted_theme actions

#33 @pbiron
5 years ago

14955.2.diff adds the delete_theme actions before the deletion attempt and the deleted_theme action after the deletion attempt.

I think #37951 should be opened back up as it is more appropriate to these hooks (since this ticket was originally intended for theme uninstall hooks), but since @vetyst was encouraged to add his similar patch to this ticket I did as well.

My use case is simple: I've got a plugin that modifies the database when themes are installed (which it detects by hooking into upgrader_process_complete) and I need it to cleanup after itself when themes are deleted.

Last edited 5 years ago by pbiron (previous) (diff)

#34 @bookdude13
4 years ago

The auto-updates feature plugin also needs this hook to clean up update metadata about themes. Even if themes shouldn't need clean-up hooks, plugins relating to themes do.

The patch applies cleanly, I move for this being added.

#35 @desrosj
3 years ago

  • Keywords dev-feedback removed
  • Resolution set to maybelater
  • Status changed from new to closed

I came across this looking for a theme equivalent to deleted_plugin hook only to find it doesn't exist yet.

14955.2.diff looks like it would solve my use case, but it does not address the original ask of the ticket. Since an active theme cannot be deleted, any hooks in the theme will not be registered, so using deleted_theme wouldn't work. I'm reopening #16401 and will use that ticket to commit these hooks.

I do see a difference between using switch_theme and waiting to delete options until the theme is actually deleted. It's not uncommon to switch themes to try a new one out before switching back if you prefer the old one. If a theme used switch_theme to remove custom settings or options, the user would lose their customizations.

That said, the purpose of and concept of what constitutes a theme is undergoing a massive shift with the introduction of full site editing in 5.8 and theme.json files. I don't know that I see a clear use case of an uninstall hook in FSE era themes yet.

With that in mind, and that this ticket has seen no meaningful movement in 5+ years, I'm going to close this out as a maybelater. Discussion can always continue on maybelater tickets, and they can be reopened once things become more clear.

Note: See TracTickets for help on using tickets.