Fixed #6037 -- Placeholders without content in edit mode should also …

…fallback.
django-cms · czpython · Sep 6, 2017 · Sep 5, 2017 · Sep 5, 2017 · beef547aa4c7123fd3f438a524e663a000f87d47
commit beef547aa4c7123fd3f438a524e663a000f87d47
diff --git a/cms/plugin_rendering.py b/cms/plugin_rendering.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 import warnings
 
-from collections import deque
+from collections import OrderedDict
 
 from functools import partial
 
@@ -48,14 +48,23 @@ def _get_page_ancestors(page):
 
 
 class RenderedPlaceholder(object):
-    __slots__ = ('placeholder', 'language', 'site_id', 'cached', 'editable')
-
-    def __init__(self, placeholder, language, site_id, cached=False, editable=False):
-        self.placeholder = placeholder
+    __slots__ = (
+        'language',
+        'site_id',
+        'cached',
+        'editable',
+        'placeholder',
+        'has_content',
+    )
+
+    def __init__(self, placeholder, language, site_id, cached=False,
+                 editable=False, has_content=False):
         self.language = language
         self.site_id = site_id
         self.cached = cached
         self.editable = editable
+        self.placeholder = placeholder
+        self.has_content = has_content
 
     def __eq__(self, other):
         # The same placeholder rendered with different
@@ -78,8 +87,8 @@ def __init__(self, request):
         self._cached_templates = {}
         self._placeholders_content_cache = {}
         self._placeholders_by_page_cache = {}
-        self._rendered_placeholders = deque()
-        self._rendered_static_placeholders = deque()
+        self._rendered_placeholders = OrderedDict()
+        self._rendered_static_placeholders = OrderedDict()
         self._rendered_plugins_by_placeholder = {}
         self._placeholders_are_editable = self.user_is_on_edit_mode()
 
@@ -146,13 +155,15 @@ def get_rendered_plugins_cache(self, placeholder):
         return self._rendered_plugins_by_placeholder.get(placeholder.pk, blank)
 
     def get_rendered_placeholders(self):
-        return [r.placeholder for r in self._rendered_placeholders]
+        rendered = list(self._rendered_placeholders.values())
+        return [r.placeholder for r in rendered]
 
     def get_rendered_editable_placeholders(self):
-        return [r.placeholder for r in self._rendered_placeholders if r.editable]
+        rendered = list(self._rendered_placeholders.values())
+        return [r.placeholder for r in rendered if r.editable]
 
     def get_rendered_static_placeholders(self):
-        return self._rendered_static_placeholders
+        return list(self._rendered_static_placeholders.values())
 
     def render_placeholder(self, placeholder, context, language=None, page=None,
                            editable=False, use_cache=False, nodelist=None, width=None):
@@ -256,17 +267,18 @@ def render_placeholder(self, placeholder, context, language=None, page=None,
             site_id=site_id,
             cached=use_cache,
             editable=editable,
+            has_content=bool(placeholder_content),
         )
 
-        if rendered_placeholder not in self._rendered_placeholders:
+        if placeholder.pk not in self._rendered_placeholders:
             # First time this placeholder is rendered
             if not self.toolbar._cache_disabled:
                 # The toolbar middleware needs to know if the response
                 # is to be cached.
                 # Set the _cache_disabled flag to the value of cache_placeholder
                 # only if the flag is False (meaning cache is enabled).
                 self.toolbar._cache_disabled = not use_cache
-            self._rendered_placeholders.append(rendered_placeholder)
+            self._rendered_placeholders[placeholder.pk] = rendered_placeholder
 
         context.pop()
         return mark_safe(toolbar_content + placeholder_content)
@@ -289,6 +301,7 @@ def render_page_placeholder(self, slot, context, inherit,
             placeholder = placeholder_cache[current_page.pk][slot]
         except KeyError:
             content = ''
+            placeholder = None
         else:
             content = self.render_placeholder(
                 placeholder,
@@ -324,7 +337,17 @@ def render_page_placeholder(self, slot, context, inherit,
                 editable=False,
             )
 
-        if not content and nodelist:
+        if placeholder and (editable and self._placeholders_are_editable):
+            # In edit mode, the contents of the placeholder are mixed with our
+            # internal toolbar markup, so the content variable will always be True.
+            # Use the rendered placeholder has_content flag instead.
+            has_content = self._rendered_placeholders[placeholder.pk].has_content
+        else:
+            # User is not in edit mode or the placeholder doesn't exist.
+            # Either way, we can trust the content variable.
+            has_content = bool(content)
+
+        if not has_content and nodelist:
             return nodelist.render(context)
         return content
 
@@ -351,9 +374,9 @@ def render_static_placeholder(self, static_placeholder, context, nodelist=None):
             nodelist=nodelist,
         )
 
-        if static_placeholder not in self._rendered_static_placeholders:
+        if static_placeholder.pk not in self._rendered_static_placeholders:
             # First time this static placeholder is rendered
-            self._rendered_static_placeholders.append(static_placeholder)
+            self._rendered_static_placeholders[static_placeholder.pk] = static_placeholder
         return content
 
     def render_plugin(self, instance, context, placeholder=None, editable=False):

diff --git a/cms/tests/test_rendering.py b/cms/tests/test_rendering.py
@@ -217,8 +217,9 @@ def strip_rendered(self, content):
         return content.strip().replace(u"\n", u"")
 
     @override_settings(CMS_TEMPLATES=[(TEMPLATE_NAME, '')])
-    def render(self, page, template=None, context_vars=None):
-        request = self.get_request(page=page)
+    def render(self, page, template=None, context_vars=None, request=None):
+        if request is None:
+            request = self.get_request(page=page)
 
         if context_vars is None:
             context_vars = {}
@@ -314,6 +315,22 @@ def test_placeholder_or(self):
         r = self.render(self.test_page, template=t)
         self.assertEqual(r, u'|No content')
 
+    def test_placeholder_or_in_edit_mode(self):
+        """
+        Tests the {% placeholder or %} templatetag in edit mode.
+        """
+        t = u'{% load cms_tags %}' + \
+            u'|{% placeholder "empty" or %}No content{% endplaceholder %}'
+        superuser = self.get_superuser()
+
+        with self.login_user_context(superuser):
+            endpoint = self.test_page.get_absolute_url() + '?edit'
+            request = self.get_request(endpoint, page=self.test_page)
+            request.session['cms_edit'] = True
+            request.toolbar = CMSToolbar(request)
+        r = self.render(self.test_page, template=t, request=request)
+        self.assertEqual(r, u'|No content')
+
     def test_render_placeholder_tag(self):
         """
         Tests the {% render_placeholder %} templatetag.