diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 58e92c9689b07..2b47ed07fa2c0 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -304,7 +304,7 @@ def non_form_errors(self): @property def media(self): - media = self.opts.media + self.formset.media + media = self.formset.media + self.opts.media for fs in self: media = media + fs.media return media diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index edd0d7c3adb2b..714a27e994038 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -579,9 +579,9 @@ def urls(self): def media(self): extra = '' if settings.DEBUG else '.min' js = [ - 'core.js', 'vendor/jquery/jquery%s.js' % extra, 'jquery.init.js', + 'core.js', 'admin/RelatedObjectLookups.js', 'actions%s.js' % extra, 'urlify.js', diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 4b932f2c01ffb..7cf71bb6ce7f9 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -4,6 +4,7 @@ import copy from django import forms +from django.conf import settings from django.db.models.deletion import CASCADE from django.urls import reverse from django.urls.exceptions import NoReverseMatch @@ -22,7 +23,14 @@ class FilteredSelectMultiple(forms.SelectMultiple): """ @property def media(self): - js = ["core.js", "SelectBox.js", "SelectFilter2.js"] + extra = '' if settings.DEBUG else '.min' + js = [ + 'vendor/jquery/jquery%s.js' % extra, + 'jquery.init.js', + 'core.js', + 'SelectBox.js', + 'SelectFilter2.js', + ] return forms.Media(js=["admin/js/%s" % path for path in js]) def __init__(self, verbose_name, is_stacked, attrs=None, choices=()): @@ -43,7 +51,13 @@ def get_context(self, name, value, attrs): class AdminDateWidget(forms.DateInput): @property def media(self): - js = ["calendar.js", "admin/DateTimeShortcuts.js"] + extra = '' if settings.DEBUG else '.min' + js = [ + 'vendor/jquery/jquery%s.js' % extra, + 'jquery.init.js', + 'calendar.js', + 'admin/DateTimeShortcuts.js', + ] return forms.Media(js=["admin/js/%s" % path for path in js]) def __init__(self, attrs=None, format=None): @@ -56,7 +70,13 @@ def __init__(self, attrs=None, format=None): class AdminTimeWidget(forms.TimeInput): @property def media(self): - js = ["calendar.js", "admin/DateTimeShortcuts.js"] + extra = '' if settings.DEBUG else '.min' + js = [ + 'vendor/jquery/jquery%s.js' % extra, + 'jquery.init.js', + 'calendar.js', + 'admin/DateTimeShortcuts.js', + ] return forms.Media(js=["admin/js/%s" % path for path in js]) def __init__(self, attrs=None, format=None): diff --git a/django/contrib/gis/admin/options.py b/django/contrib/gis/admin/options.py index 431f138f1bb2c..fe74df36ff429 100644 --- a/django/contrib/gis/admin/options.py +++ b/django/contrib/gis/admin/options.py @@ -2,6 +2,7 @@ from django.contrib.gis.admin.widgets import OpenLayersWidget from django.contrib.gis.db import models from django.contrib.gis.gdal import OGRGeomType +from django.forms import Media spherical_mercator_srid = 3857 @@ -46,10 +47,7 @@ class GeoModelAdmin(ModelAdmin): @property def media(self): "Injects OpenLayers JavaScript into the admin." - media = super().media - media.add_js([self.openlayers_url]) - media.add_js(self.extra_js) - return media + return super().media + Media(js=[self.openlayers_url] + self.extra_js) def formfield_for_dbfield(self, db_field, request, **kwargs): """ diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 01ada423c9346..93f831e6cc059 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -5,6 +5,7 @@ import copy import datetime import re +import warnings from contextlib import suppress from itertools import chain @@ -33,19 +34,23 @@ MEDIA_TYPES = ('css', 'js') +class MediaOrderConflictWarning(RuntimeWarning): + pass + + @html_safe class Media: - def __init__(self, media=None, **kwargs): - if media: - media_attrs = media.__dict__ + def __init__(self, media=None, css=None, js=None): + if media is not None: + css = getattr(media, 'css', {}) + js = getattr(media, 'js', []) else: - media_attrs = kwargs - - self._css = {} - self._js = [] - - for name in MEDIA_TYPES: - getattr(self, 'add_' + name)(media_attrs.get(name)) + if css is None: + css = {} + if js is None: + js = [] + self._css = css + self._js = js def __str__(self): return self.render() @@ -88,24 +93,48 @@ def __getitem__(self, name): return Media(**{str(name): getattr(self, '_' + name)}) raise KeyError('Unknown media type "%s"' % name) - def add_js(self, data): - if data: - for path in data: - if path not in self._js: - self._js.append(path) + @staticmethod + def merge(list_1, list_2): + """ + Merge two lists while trying to keep the relative order of the elements. + Warn if the lists have the same two elements in a different relative + order. - def add_css(self, data): - if data: - for medium, paths in data.items(): - for path in paths: - if not self._css.get(medium) or path not in self._css[medium]: - self._css.setdefault(medium, []).append(path) + For static assets it can be important to have them included in the DOM + in a certain order. In JavaScript you may not be able to reference a + global or in CSS you might want to override a style. + """ + # Start with a copy of list_1. + combined_list = list(list_1) + last_insert_index = len(list_1) + # Walk list_2 in reverse, inserting each element into combined_list if + # it doesn't already exist. + for path in reversed(list_2): + try: + # Does path already exist in the list? + index = combined_list.index(path) + except ValueError: + # Add path to combined_list since it doesn't exist. + combined_list.insert(last_insert_index, path) + else: + if index > last_insert_index: + warnings.warn( + 'Detected duplicate Media files in an opposite order:\n' + '%s\n%s' % (combined_list[last_insert_index], combined_list[index]), + MediaOrderConflictWarning, + ) + # path already exists in the list. Update last_insert_index so + # that the following elements are inserted in front of this one. + last_insert_index = index + return combined_list def __add__(self, other): combined = Media() - for name in MEDIA_TYPES: - getattr(combined, 'add_' + name)(getattr(self, '_' + name, None)) - getattr(combined, 'add_' + name)(getattr(other, '_' + name, None)) + 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() + } return combined diff --git a/docs/releases/2.0.txt b/docs/releases/2.0.txt index 81162e549f369..d90b80124d4bb 100644 --- a/docs/releases/2.0.txt +++ b/docs/releases/2.0.txt @@ -554,6 +554,11 @@ Miscellaneous * Renamed ``BaseExpression._output_field`` to ``output_field``. You may need to update custom expressions. +* In older versions, forms and formsets combine their ``Media`` with widget + ``Media`` by concatenating the two. The combining now tries to :ref:`preserve + the relative order of elements in each list `. + ``MediaOrderConflictWarning`` is issued if the order can't be preserved. + .. _deprecated-features-2.0: Features deprecated in 2.0 diff --git a/docs/topics/forms/media.txt b/docs/topics/forms/media.txt index 0bdf237a21d16..b98cc9e740b68 100644 --- a/docs/topics/forms/media.txt +++ b/docs/topics/forms/media.txt @@ -305,6 +305,42 @@ specified by both:: +.. _form-media-asset-order: + +Order of assets +--------------- + +The order in which assets are inserted into the DOM if often important. For +example, you may have a script that depends on jQuery. Therefore, combining +``Media`` objects attempts to preserve the relative order in which assets are +defined in each ``Media`` class. + +For example:: + + >>> from django import forms + >>> class CalendarWidget(forms.TextInput): + ... class Media: + ... js = ('jQuery.js', 'calendar.js', 'noConflict.js') + >>> class TimeWidget(forms.TextInput): + ... class Media: + ... js = ('jQuery.js', 'time.js', 'noConflict.js') + >>> w1 = CalendarWidget() + >>> w2 = TimeWidget() + >>> print(w1.media + w2.media) + + + + + +Combining ``Media`` objects with assets in a conflicting order results in a +``MediaOrderConflictWarning``. + +.. versionchanged:: 2.0 + + In older versions, the assets of ``Media`` objects are concatenated rather + than merged in a way that tries to preserve the relative ordering of the + elements in each list. + ``Media`` on Forms ================== diff --git a/tests/forms_tests/tests/test_media.py b/tests/forms_tests/tests/test_media.py index 488fe190b2755..dd10c60d2ae6c 100644 --- a/tests/forms_tests/tests/test_media.py +++ b/tests/forms_tests/tests/test_media.py @@ -1,3 +1,5 @@ +import warnings + from django.forms import CharField, Form, Media, MultiWidget, TextInput from django.template import Context, Template from django.test import SimpleTestCase, override_settings @@ -106,7 +108,7 @@ class Media: class MyWidget3(TextInput): class Media: css = { - 'all': ('/path/to/css3', 'path/to/css1') + 'all': ('path/to/css1', '/path/to/css3') } js = ('/path/to/js1', '/path/to/js4') @@ -237,9 +239,9 @@ class Media: w8 = MyWidget8() self.assertEqual( str(w8.media), - """ + """ + - @@ -312,9 +314,9 @@ class Media: w11 = MyWidget11() self.assertEqual( str(w11.media), - """ + """ + - @@ -341,9 +343,9 @@ class Media: w12 = MyWidget12() self.assertEqual( str(w12.media), - """ + """ + - """ ) @@ -396,7 +398,7 @@ class Media: class MyWidget3(TextInput): class Media: css = { - 'all': ('/path/to/css3', 'path/to/css1') + 'all': ('path/to/css1', '/path/to/css3') } js = ('/path/to/js1', '/path/to/js4') @@ -441,7 +443,7 @@ class Media: class MyWidget3(TextInput): class Media: css = { - 'all': ('/path/to/css3', 'path/to/css1') + 'all': ('path/to/css1', '/path/to/css3') } js = ('/path/to/js1', '/path/to/js4') @@ -518,3 +520,25 @@ def test_html_safe(self): media = Media(css={'all': ['/path/to/css']}, js=['/path/to/js']) self.assertTrue(hasattr(Media, '__html__')) self.assertEqual(str(media), media.__html__()) + + def test_merge(self): + test_values = ( + (([1, 2], [3, 4]), [1, 2, 3, 4]), + (([1, 2], [2, 3]), [1, 2, 3]), + (([2, 3], [1, 2]), [1, 2, 3]), + (([1, 3], [2, 3]), [1, 2, 3]), + (([1, 2], [1, 3]), [1, 2, 3]), + (([1, 2], [3, 2]), [1, 3, 2]), + ) + for (list1, list2), expected in test_values: + with self.subTest(list1=list1, list2=list2): + self.assertEqual(Media.merge(list1, list2), expected) + + def test_merge_warning(self): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + self.assertEqual(Media.merge([1, 2], [2, 1]), [1, 2]) + self.assertEqual( + str(w[-1].message), + 'Detected duplicate Media files in an opposite order:\n1\n2' + )