Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#52817 closed defect (bug) (fixed)

hello dolly does not delete the language file

Reported by: otshelnik-fm's profile Otshelnik-Fm Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.7
Component: Plugins Keywords: has-patch needs-testing
Focuses: administration Cc:

Description

When deleting the Hello Dolly plugin (i uninstall the plugin through the plugin uninstall interface), the translation file under the path /wp-content/languages/plugins - is not deleted

hello-dolly-ru_RU.mo
hello-dolly-ru_RU.po
— these files remained when we deleted Hello Dolly.

I believe that when you delete a plugin, its files outside the plugin directory should also be deleted.

Change History (9)

#1 @SergeyBiryukov
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.8

Thanks for the ticket!

Generally, plugin translations are deleted along with the plugin, see [29856] / #29860.

It looks like Hello Dolly is a bit of an edge case here, as the file name (hello.php) does not match the plugin slug in the directory (hello-dolly). If you delete the plugin, reinstall it from the directory, and delete again, I believe that would delete the translation as well :)

There is an exception for Hello Dolly in _get_plugin_data_markup_translate(), added in [19965] / #19597, to make sure its translation is loaded correctly.

Perhaps another exception is needed in delete_plugins(). Or, maybe somewhere else too, so that the translation would only be downloaded when installing the plugin from the directory, since otherwise a hello-dolly-ru_RU.po file is simply redundant and unused, as the translation for Hello Dolly is already included in WordPress core language files (admin-ru_RU.po in this case).

Just a reminder for anyone following the ticket, that this is not the place to suggest removing Hello Dolly from core, see #11538 for that.

Some other tickets that may (or may not) also be related: #29540, #37351, and #49338.

#2 @JeffPaul
4 years ago

  • Milestone changed from 5.8 to Future Release

Good details on approach above, but with no patch/PR and with 5.8 Beta 1 less than a week away I'm going to punt this to Future Release. Once a patch/PR is available we can look to move this back to a numbered milestone.

This ticket was mentioned in PR #1327 on WordPress/wordpress-develop by costdev.


4 years ago
#3

  • Keywords has-patch added; needs-patch removed

delete_plugins() tests two conditions:

1) That the $plugin_slug is not equal to ..
2) That $plugin_translations[ $plugin_slug ] is not empty().

As Hello Dolly does not have its own directory, its $plugin_slug is ..
The first condition returns false and the translations files for Hello Dolly remain in wp-content/languages/plugins.

This edge case is now rectified by a conditional which specifically checks if $plugin_file is hello.php and correctly assigns $plugin_slug to hello-dolly.

Fixes #52817.

Trac ticket: https://core.trac.wordpress.org/ticket/52817

#4 @costdev
4 years ago

  • Keywords needs-patch added; has-patch removed

Hi @JeffPaul - I've just submitted a PR for this ticket.

While at this time, this issue exists as an edge case with Hello Dolly, it could theoretically occur for any plugin which isn't in its own directory in wp-content/plugins.

As I'm not sure how often this is likely to occur, the PR I've submitted resolves this specific edge case for Hello Dolly.

#5 @costdev
4 years ago

  • Keywords has-patch added; needs-patch removed

Apologies, for the has-patch removal - The page had updated but I hadn't refreshed and the properties still had needs-patch applied.

#6 @JeffPaul
4 years ago

  • Keywords needs-testing added

Thanks @costdev, I'll mention this in today's devchat open floor in case someone's able to review ahead of the 5.8 Beta 1 this coming Tuesday.

#7 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 5.8

#8 @SergeyBiryukov
4 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 51064:

Plugins: Make sure Hello Dolly translations are deleted when the plugin is deleted.

Follow-up to [19965], [29856].

Props costdev, Otshelnik-Fm, JeffPaul, SergeyBiryukov.
Fixes #52817.

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


4 years ago

Note: See TracTickets for help on using tickets.