Browse Source

Fixed #30153 -- Fixed incorrect form Media asset ordering after three way merge.

Delaying merging assets as long as possible avoids introducing
incorrect relative orderings that cause a broken final result.
Matthias Kestenholz 6 years ago
parent
commit
959d0c078a
2 changed files with 53 additions and 7 deletions
  1. 23 7
      django/forms/widgets.py
  2. 30 0
      tests/forms_tests/tests/test_media.py

+ 23 - 7
django/forms/widgets.py

@@ -48,8 +48,8 @@ class Media:
                 css = {}
             if js is None:
                 js = []
-        self._css = css
-        self._js = js
+        self._css_lists = [css]
+        self._js_lists = [js]
 
     def __repr__(self):
         return 'Media(css=%r, js=%r)' % (self._css, self._js)
@@ -57,6 +57,25 @@ class Media:
     def __str__(self):
         return self.render()
 
+    @property
+    def _css(self):
+        css = self._css_lists[0]
+        # filter(None, ...) avoids calling merge with empty dicts.
+        for obj in filter(None, self._css_lists[1:]):
+            css = {
+                medium: self.merge(css.get(medium, []), obj.get(medium, []))
+                for medium in css.keys() | obj.keys()
+            }
+        return css
+
+    @property
+    def _js(self):
+        js = self._js_lists[0]
+        # filter(None, ...) avoids calling merge() with empty lists.
+        for obj in filter(None, self._js_lists[1:]):
+            js = self.merge(js, obj)
+        return js
+
     def render(self):
         return mark_safe('\n'.join(chain.from_iterable(getattr(self, 'render_' + name)() for name in MEDIA_TYPES)))
 
@@ -132,11 +151,8 @@ class Media:
 
     def __add__(self, other):
         combined = Media()
-        combined._js = self.merge(self._js, other._js)
-        combined._css = {
-            medium: self.merge(self._css.get(medium, []), other._css.get(medium, []))
-            for medium in self._css.keys() | other._css.keys()
-        }
+        combined._css_lists = self._css_lists + other._css_lists
+        combined._js_lists = self._js_lists + other._js_lists
         return combined
 
 

+ 30 - 0
tests/forms_tests/tests/test_media.py

@@ -541,3 +541,33 @@ class FormsMediaTestCase(SimpleTestCase):
         msg = 'Detected duplicate Media files in an opposite order:\n1\n2'
         with self.assertWarnsMessage(RuntimeWarning, msg):
             self.assertEqual(Media.merge([1, 2], [2, 1]), [1, 2])
+
+    def test_merge_js_three_way(self):
+        """
+        The relative order of scripts is preserved in a three-way merge.
+        """
+        # custom_widget.js doesn't depend on jquery.js.
+        widget1 = Media(js=['custom_widget.js'])
+        widget2 = Media(js=['jquery.js', 'uses_jquery.js'])
+        form_media = widget1 + widget2
+        # The relative ordering of custom_widget.js and jquery.js has been
+        # established (but without a real need to).
+        self.assertEqual(form_media._js, ['custom_widget.js', 'jquery.js', 'uses_jquery.js'])
+        # The inline also uses custom_widget.js. This time, it's at the end.
+        inline_media = Media(js=['jquery.js', 'also_jquery.js']) + Media(js=['custom_widget.js'])
+        merged = form_media + inline_media
+        self.assertEqual(merged._js, ['custom_widget.js', 'jquery.js', 'uses_jquery.js', 'also_jquery.js'])
+
+    def test_merge_css_three_way(self):
+        widget1 = Media(css={'screen': ['a.css']})
+        widget2 = Media(css={'screen': ['b.css']})
+        widget3 = Media(css={'all': ['c.css']})
+        form1 = widget1 + widget2
+        form2 = widget2 + widget1
+        # form1 and form2 have a.css and b.css in different order...
+        self.assertEqual(form1._css, {'screen': ['a.css', 'b.css']})
+        self.assertEqual(form2._css, {'screen': ['b.css', 'a.css']})
+        # ...but merging succeeds as the relative ordering of a.css and b.css
+        # was never specified.
+        merged = widget3 + form1 + form2
+        self.assertEqual(merged._css, {'screen': ['a.css', 'b.css'], 'all': ['c.css']})