Skip to content


Subversion checkout URL

You can clone with
Download ZIP


make sure 'cancel' does not create empty plugins #1562

wants to merge 3 commits into from

3 participants


javascript prevented the form from being sent if 'cancel' was clicked, and therefore plugins were not deleted in the cleanup code (only affects frontend editing).
see issue #861


This fix only seems to address "active" cancels but as far as I can see doesn't actually fix the issue of empty plugins ending up in the tree, as users can cancel by closing the browser window or by clicking outside the overlay. To properly fix this we need to not save the plugin until the user clicks the save button.

The proposed patch might be useful in the short term but unfortunately doesn't seem to fix the actual issue.


There was an issue why we created the CMSPlugin first and only after that the real plugin... it was the same with the page. But i can't remember exactly why :)


I propose that we do the cleanup as well when you hit publish.... go over all plugins and look if there are empty ones and delete them. This would fix the second issue.


that's still just fighting symptoms. This can't be impossible to properly fix in the add mechanism. Pages don't have this issue, if you create a page, a proper page is created. With plugins, we first create the base plugin and then call 'edit' on the concrete plugin class. Why is it impossible to just call 'add' on the concrete plugin class? We already know what type we want to create...


I'll close this for now, since I'm really strongly against this approach. See my branch

@ojii ojii closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 19, 2012
  1. make sure 'cancel' does not create empty plugins

    Pascal Mouret committed
Commits on Dec 20, 2012
  1. cleanup method & cleanup on publish (empty plugins)

    Pascal Mouret committed
  2. added a test for plugin cleanup

    Pascal Mouret committed
This page is out of date. Refresh to see the latest.
2  cms/admin/
@@ -1283,7 +1283,7 @@ def edit_plugin(self, request, plugin_id):
instance = cms_plugin.get_plugin_instance()[0]
if not instance:
# cancelled before any content was added to plugin
- cms_plugin.delete()
+ cms_plugin.delete_with_public()
4 cms/models/
@@ -9,7 +9,7 @@
from cms.utils import i18n, urlutils, page as page_utils
from cms.utils import timezone
from cms.utils.copy_plugins import copy_plugins_to
-from cms.utils.helpers import reversion_register
+from cms.utils.helpers import reversion_register, cleanup_plugins
from django.conf import settings
from django.contrib.sites.models import Site
from django.core.urlresolvers import reverse
@@ -447,6 +447,8 @@ def publish(self):
elif page.publisher_state == Page.PUBLISHER_STATE_PENDING:
+ cleanup_plugins(page=self)
# fire signal after publishing is done
import cms.signals as cms_signals
cms_signals.post_publish.send(sender=Page, instance=self)
4 cms/static/cms/js/plugins/cms.placeholders.js
@@ -144,10 +144,6 @@ CMS.$(document).ready(function ($) {
var cancel = $(this).contents().find('input[name^="_cancel"]');
cancel.bind('click', function (e) {
- e.preventDefault();
- // hide frame
- that.toggleFrame();
- that.toggleDim();
// If the toolbar was hidden before we started editing
// this plugin, and it is NOT hidden now, hide it
if (needs_collapsing && ! CMS.API.Toolbar.isToolbarHidden()){
23 cms/tests/
@@ -364,12 +364,23 @@ def test_remove_plugin_after_published(self):
+ 'body': '<p>text</p>'
response =, plugin_data)
plugin_id = int(response.content)
self.assertEquals(response.status_code, 200)
self.assertEquals(int(response.content), CMSPlugin.objects.all()[0].pk)
+ # add some stuff to the plugin, so it won't get cleaned up on publish
+ edit_url = URL_CMS_PLUGIN_EDIT + response.content + "/"
+ response = self.client.get(edit_url)
+ self.assertEquals(response.status_code, 200)
+ data = {
+ "body":'<p>We\'re all going to die!</p>'
+ }
+ response =, data)
+ self.assertEquals(response.status_code, 200)
# there should be only 1 plugin
self.assertEquals(CMSPlugin.objects.all().count(), 1)
self.assertEquals(CMSPlugin.objects.filter(placeholder__page__publisher_is_draft=True).count(), 1)
@@ -698,6 +709,18 @@ def test_moving_plugin_to_different_placeholder(self):
+ def test_empty_plugin_cleanup(self):
+ # check if plugin gets deleted during publish
+ page = create_page('page', 'nav_playground.html', 'en', published=False)
+ self._create_text_plugin_on_page(page)
+ page.publish()
+ self.assertEquals(CMSPlugin.objects.all().count(), 0)
+ # check if a filled plugin is left alone
+ pid = self._create_text_plugin_on_page(page)
+ self._edit_text_plugin(pid, '<p>So long, and thanks for all the fish.</p>')
+ page.publish()
+ self.assertEquals(CMSPlugin.objects.all().count(), 2)
class FileSystemPluginTests(PluginsTestBaseCase):
def setUp(self):
23 cms/utils/
@@ -66,4 +66,25 @@ def make_revision_with_plugins(obj, user=None, message=None):
revision_context.add_to_context(revision_manager, plugin, bpadapter.get_version_data(plugin, VERSION_CHANGE))
def find_placeholder_relation(obj):
- return 'page'
+ return 'page'
+def cleanup_plugins(page=None, placeholder=None, pluginlist=[]):
+ '''
+ Delete all <Empty> plugins in the given placeholder/page/list of plugins.
+ '''
+ plugins = pluginlist
+ if placeholder:
+ plugins = placeholder.get_plugins_list()
+ if page:
+ collection = []
+ placeholders = page.placeholders
+ for placeholder in placeholders.all():
+ collection += placeholder.get_plugins_list()
+ plugins = collection
+ for plugin in plugins:
+ if not plugin.get_plugin_instance()[0]:
+ plugin.delete_with_public()
Something went wrong with that request. Please try again.