From 959d0c078a1c903cd1e4850932be77c4f0d2294d Mon Sep 17 00:00:00 2001 From: Matthias Kestenholz Date: Sat, 9 Feb 2019 15:38:52 +0100 Subject: [PATCH] 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. --- django/forms/widgets.py | 30 ++++++++++++++++++++------- tests/forms_tests/tests/test_media.py | 30 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/django/forms/widgets.py b/django/forms/widgets.py index eed1fa5c3b..02aa32b207 100644 --- a/django/forms/widgets.py +++ b/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 diff --git a/tests/forms_tests/tests/test_media.py b/tests/forms_tests/tests/test_media.py index a586da90c5..8cb484a15e 100644 --- a/tests/forms_tests/tests/test_media.py +++ b/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']})