#30179: Merging 3 or more media objects can throw unnecessary
MediaOrderConflictWarnings
-------------------------------+------------------------------------
Reporter: Matt Westcott | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by Matthias Kestenholz):
So
https://github.com/django/django/commit/959d0c078a1c903cd1e4850932be77c4f0d2294d
(the fix for #30153) didn't make this case worse, it just didn't improve
on it. The problem is actually the same I encountered, with the same
unintuitive error message too. There is still a way to produce a
conflicting order but it's harder to trigger in the administration
interface now but unfortunately still easy. Also, going back to the state
of things pre 2.0 was already discussed previously and rejected.
Here's a failing test and and an idea to make this particular test pass:
Merge the JS sublists starting from the longest list and continuing with
shorter lists. The CSS case is missing yet. The right thing to do would be
(against [http://wiki.c2.com/?WorseIsBetter worse is better]) to add some
sort of dependency resolution solver with backtracking but that's surely a
bad idea for many other reasons.
The change makes some old tests fail (I only took a closer look at
`test_merge_js_three_way` and in this case the failure is fine --
`custom_widget.js` is allowed to appear before `jquery.js`.)
{{{
diff --git a/django/forms/widgets.py b/django/forms/widgets.py
index 02aa32b207..d85c409152 100644
--- a/django/forms/widgets.py
+++ b/django/forms/widgets.py
@@ -70,9 +70,15 @@ class Media:
@property
def _js(self):
- js = self._js_lists[0]
+ sorted_by_length = list(sorted(
+ filter(None, self._js_lists),
+ key=lambda lst: -len(lst),
+ ))
+ if not sorted_by_length:
+ return []
+ js = sorted_by_length[0]
# filter(None, ...) avoids calling merge() with empty lists.
- for obj in filter(None, self._js_lists[1:]):
+ for obj in filter(None, sorted_by_length[1:]):
js = self.merge(js, obj)
return js
diff --git a/tests/forms_tests/tests/test_media.py
b/tests/forms_tests/tests/test_media.py
index 8cb484a15e..9d17ad403b 100644
--- a/tests/forms_tests/tests/test_media.py
+++ b/tests/forms_tests/tests/test_media.py
@@ -571,3 +571,12 @@ class FormsMediaTestCase(SimpleTestCase):
# was never specified.
merged = widget3 + form1 + form2
self.assertEqual(merged._css, {'screen': ['a.css', 'b.css'],
'all': ['c.css']})
+
+ def test_merge_js_some_more(self):
+ widget1 = Media(js=['color-picker.js'])
+ widget2 = Media(js=['text-editor.js'])
+ widget3 = Media(js=['text-editor.js', 'text-editor-extras.js',
'color-picker.js'])
+
+ merged = widget1 + widget2 + widget3
+
+ self.assertEqual(merged._js, ['text-editor.js', 'text-editor-
extras.js', 'color-picker.js'])
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30179#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/064.94a22ff20b9f90ede867d2ed7d246d5a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
