ModelAdmin with custom widgets, inlines, and filter_horizontal can merge media in broken order

[roybi]

when a modeadmin have a inline with a filed has its own media js no need jquery , and have a one to many field show filter_horizontal, the problem appear.
there will be MediaOrderConflictWarning and inlines.js load before jquery.

[roybi]

This is the test program

[Tim Graham]

I dove into this for a while. I'm not sure if there's anything that Django can do about it. Reverting 03974d81220ffd237754a82c77913799dd5909a4 solves the problem but that'll break other cases.

The root causes seems to be that ckeditor/ckeditor/ckeditor.js is collected before jquery in one case and after it in another.

[Matthias Kestenholz]

It seems to me that Django assumes too much. Here are some assets that have been collected in my case:

[
'imagefield/ppoi.js', 
<JS(ckeditor/ckeditor-init.js, {'id': 'ckeditor-init-script', 'data-ckeditor-basepath': '/static/ckeditor/ckeditor/'})>, 
'ckeditor/ckeditor/ckeditor.js', 
'admin/js/vendor/jquery/jquery.js', 
'admin/js/jquery.init.js', 
'admin/js/core.js', 
'admin/js/admin/RelatedObjectLookups.js', 
'admin/js/actions.js', 
'admin/js/urlify.js', 
'admin/js/prepopulate.js', 
'admin/js/vendor/xregexp/xregexp.js', 
<JS(https://use.fontawesome.com/releases/v5.3.1/js/all.js, {'async': 'async', 'integrity': 'sha384-kW+oWsYx3YpxvjtZjFXqazFpA7UP/MbiY4jvs+RWZo2+N94PFZ36T6TFkc9O3qoB', 'crossorigin': 'anonymous'})>, 
'app/plugin_buttons.js', 
'admin/js/calendar.js', 
'admin/js/admin/DateTimeShortcuts.js'
]

The imagefield and ckeditor assets are there because of widgets. When using the same widgets in inlines the inlines' Media class will contain jquery, jquery.init.js, inlines.js, imagefield/ppoi.js. When merging the two JS lists Django will find that imagefield/ppoi.js is at index 0, will continue with inlines.js (and of course not find it) and insert it at index 0 as well (because that's the value of last_insert_index now). As soon as jquery.init.js is encountered it notices that something is amiss and emits a MediaOrderConflictWarning. The problem was produced one iteration earlier and the error message is not very helpful.

I don't have a good suggestion yet. It also baffles me that only one of two candidate models/modeladmins shows the problem, and (for now) luckily only in development.

[Matthias Kestenholz]

Following up. This problem exists since Django 2.0. It doesn't show itself in Django 1.11.x (not surprising since the MediaOrderConflictWarning change was introduced with Django 2.0). It's good that I'm able to reproduce this on a different computer, so that gives me hope that it's not impossible to debug & fix.

It's unfortunate that the media files which cause the reordering don't even depend on jQuery.

[Johannes Maron]

Hi there, I was the one introducing the warning. The warning is emitted when the defined asset order can not be maintained. Which is a good thing to warn about, but not always a problem. It is particularly not an issue, if you have nested forms, where assets from multiple fields are merged into preliminary forms (like inlines) just to be merged again.
I think a real solution could be to retain information about the explicitly defined order-constraints and ignore the implicit once. The merging algorithm can stay as is, it is correct. All that would need to be done, is order violation violates an implicit or explicit asset order before emitting the warning.
This would however use a bit more memory, since we would need to keep a record of constraints (list of tuples).

What do you think? I could invest a bit of time, to draft a solution.

[Matthias Kestenholz]

Hey, yes that might be the right thing to do but maybe there is a solution which requires less effort. I suspect that the problem is an inconsistency in the way Django collects media from different places. I inserted a few print() statements here ​https://github.com/django/django/blob/893b80d95dd76642e478893ba6d4f46bb31388f1/django/contrib/admin/options.py#L1595

(Sorry for the big blob in advance)

self.media                                                                                                                                                                                                                     
 <script type="text/javascript" src="/static/admin/js/vendor/jquery/jquery.js"></script>                                                                                                                                       
<script type="text/javascript" src="/static/admin/js/jquery.init.js"></script>                                                                                                                                                 
<script type="text/javascript" src="/static/admin/js/core.js"></script>                                                                                                                                                        
<script type="text/javascript" src="/static/admin/js/admin/RelatedObjectLookups.js"></script>                                                                                                                                  
<script type="text/javascript" src="/static/admin/js/actions.js"></script>                                                                                                                                                     
<script type="text/javascript" src="/static/admin/js/urlify.js"></script>                                                                                                                                                      
<script type="text/javascript" src="/static/admin/js/prepopulate.js"></script>
<script type="text/javascript" src="/static/admin/js/vendor/xregexp/xregexp.js"></script>
<script type="text/javascript" src="https://use.fontawesome.com/releases/v5.3.1/js/all.js" async="async" crossorigin="anonymous" integrity="sha384-kW+oWsYx3YpxvjtZjFXqazFpA7UP/MbiY4jvs+RWZo2+N94PFZ36T6TFkc9O3qoB"></script>
<script type="text/javascript" src="/static/app/plugin_buttons.js"></script>

adminForm.media
 <link href="/static/imagefield/ppoi.css" type="text/css" media="screen" rel="stylesheet">
<script type="text/javascript" src="/static/imagefield/ppoi.js"></script>
<script type="text/javascript" src="/static/ckeditor/ckeditor-init.js" data-ckeditor-basepath="/static/ckeditor/ckeditor/" id="ckeditor-init-script"></script>
<script type="text/javascript" src="/static/ckeditor/ckeditor/ckeditor.js"></script>
<script type="text/javascript" src="/static/admin/js/vendor/jquery/jquery.js"></script>
<script type="text/javascript" src="/static/admin/js/jquery.init.js"></script>
<script type="text/javascript" src="/static/admin/js/calendar.js"></script>
<script type="text/javascript" src="/static/admin/js/admin/DateTimeShortcuts.js"></script>

inline_formset.media <class 'app.articles.models.RichText'>
 <script type="text/javascript" src="/static/admin/js/vendor/jquery/jquery.js"></script>
<script type="text/javascript" src="/static/admin/js/jquery.init.js"></script>
<script type="text/javascript" src="/static/admin/js/inlines.js"></script>
<script type="text/javascript" src="/static/feincms3/plugin_ckeditor.js"></script>
<script type="text/javascript" src="/static/ckeditor/ckeditor-init.js" data-ckeditor-basepath="/static/ckeditor/ckeditor/" id="ckeditor-init-script"></script>
<script type="text/javascript" src="/static/ckeditor/ckeditor/ckeditor.js"></script>

inline_formset.media <class 'app.articles.models.Image'>
 <link href="/static/imagefield/ppoi.css" type="text/css" media="screen" rel="stylesheet">
<script type="text/javascript" src="/static/admin/js/vendor/jquery/jquery.js"></script>
<script type="text/javascript" src="/static/admin/js/jquery.init.js"></script>
<script type="text/javascript" src="/static/admin/js/inlines.js"></script>
<script type="text/javascript" src="/static/imagefield/ppoi.js"></script>

So somehow the widget media files (imagefield and ckeditor) come after the files added by Django's inlines in inline formsets but before them in the helpers.AdminForm belonging to the ArticleAdmin class.

The problem manifests itself when having any third-party widget (which does not reference Django's jquery asset) before any Django date field, prepopulated field or filter_* field in the fieldsets structure. Reordering fieldsets (or fields) avoids the problem completely. Now I still don't understand why the exact same project would work on the server and not locally.

The problem can be worked around by including "admin/js/vendor/jquery/jquery%s.js", "admin/js/jquery.init.js" in third-party widgets' Media definitions. This sucks big time though, especially since those widgets don't even require jQuery to work.

jQuery is included on all modeladmin pages anyway, so a good fix might be to remove the jquery and jquery.init.js entries from admin widgets Media definitions (which makes them incomplete when used outside of Django's admin panel, but that's probably not the intention anyway) or make the Media merging algorithm aware of libraries which are supposed to always come first.

Just for reference, here's the form class and its fields. Putting e.g. publication_date before image makes everything work fine.

form.media
<link href="/static/imagefield/ppoi.css" type="text/css" media="screen" rel="stylesheet">
<script type="text/javascript" src="/static/imagefield/ppoi.js"></script>
<script type="text/javascript" src="/static/ckeditor/ckeditor-init.js" data-ckeditor-basepath="/static/ckeditor/ckeditor/" id="ckeditor-init-script"></script>
<script type="text/javascript" src="/static/ckeditor/ckeditor/ckeditor.js"></script>
<script type="text/javascript" src="/static/admin/js/vendor/jquery/jquery.js"></script>
<script type="text/javascript" src="/static/admin/js/jquery.init.js"></script>
<script type="text/javascript" src="/static/admin/js/calendar.js"></script>
<script type="text/javascript" src="/static/admin/js/admin/DateTimeShortcuts.js"></script>

form.fields
is_active: <class 'django.forms.fields.BooleanField'>
title: <class 'django.forms.fields.CharField'>
excerpt: <class 'django.forms.fields.CharField'>
image: <class 'django.forms.fields.ImageField'>
image_ppoi: <class 'django.forms.fields.CharField'>
meta_title: <class 'django.forms.fields.CharField'>
meta_description: <class 'django.forms.fields.CharField'>
meta_image: <class 'django.forms.fields.ImageField'>
meta_canonical: <class 'django.forms.fields.URLField'>
meta_author: <class 'django.forms.fields.CharField'>
meta_robots: <class 'django.forms.fields.CharField'>
show_teaser_need: <class 'django.forms.fields.BooleanField'>
show_teaser_competency: <class 'django.forms.fields.BooleanField'>
teaser_title: <class 'django.forms.fields.CharField'>
teaser_text_need: <class 'ckeditor.fields.RichTextFormField'>
teaser_text_competency: <class 'ckeditor.fields.RichTextFormField'>
teaser_image: <class 'django.forms.fields.ImageField'>
teaser_image_ppoi: <class 'django.forms.fields.CharField'>
slug: <class 'django.forms.fields.SlugField'>
publication_date: <class 'django.forms.fields.SplitDateTimeField'>
is_featured: <class 'django.forms.fields.BooleanField'>
author: <class 'django.forms.models.ModelChoiceField'>
categories: <class 'django.forms.models.ModelMultipleChoiceField'>

[Matthias Kestenholz]

I just attached a minimal patch demonstrating the problem: https://code.djangoproject.com/attachment/ticket/30153/test.patch

Simply reordering date and dummy in Holder3 makes the problem go away in this case (since Holder3's modeladmin class does not define fieldsets)

[Matthias Kestenholz]

Test showing the problem

[Matthias Kestenholz]

Replacing the patch with https://code.djangoproject.com/attachment/ticket/30153/test.2.patch because Django 2.2 and 3 will not use jquery in date fields anymore and therefore cannot be used to demonstrate the problem. I changed the code to use a filter_horizontal widget and now it fails again:

======================================================================
ERROR: test_inline_media_only_inline (admin_inlines.tests.TestInlineMedia)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.6/unittest/case.py", line 605, in run
    testMethod()
  File "/home/matthias/Projects/django/tests/admin_inlines/tests.py", line 495, in test_inline_media_only_inline
    response = self.client.get(change_url)
  File "/home/matthias/Projects/django/django/test/client.py", line 535, in get
    response = super().get(path, data=data, secure=secure, **extra)
  File "/home/matthias/Projects/django/django/test/client.py", line 347, in get
    **extra,
  File "/home/matthias/Projects/django/django/test/client.py", line 422, in generic
    return self.request(**r)
  File "/home/matthias/Projects/django/django/test/client.py", line 503, in request
    raise exc_value
  File "/home/matthias/Projects/django/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/home/matthias/Projects/django/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/matthias/Projects/django/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/matthias/Projects/django/django/contrib/admin/options.py", line 604, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/home/matthias/Projects/django/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/matthias/Projects/django/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/home/matthias/Projects/django/django/contrib/admin/sites.py", line 223, in inner
    return view(request, *args, **kwargs)
  File "/home/matthias/Projects/django/django/contrib/admin/options.py", line 1635, in change_view
    return self.changeform_view(request, object_id, form_url, extra_context)
  File "/home/matthias/Projects/django/django/utils/decorators.py", line 45, in _wrapper
    return bound_method(*args, **kwargs)
  File "/home/matthias/Projects/django/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/matthias/Projects/django/django/contrib/admin/options.py", line 1520, in changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "/home/matthias/Projects/django/django/contrib/admin/options.py", line 1594, in _changeform_view
    media = media + inline_formset.media
  File "/home/matthias/Projects/django/django/forms/widgets.py", line 135, in __add__
    combined._js = self.merge(self._js, other._js)
  File "/home/matthias/Projects/django/django/forms/widgets.py", line 126, in merge
    MediaOrderConflictWarning,
django.forms.widgets.MediaOrderConflictWarning: Detected duplicate Media files in an opposite order:
admin/js/inlines.min.js
admin/js/jquery.init.js

[Matthias Kestenholz]

@codingjoe Here's a partial fix for the issue -- ​https://github.com/matthiask/django/commit/0640ba9f5f6272987b77c35d5ad992844d6a8822

Preserving the JavaScript lists longer and only merging them at the end works. Or maybe you have a better idea? Feel free to take over if you do (or even if you don't -- if you want to)

[Matthias Kestenholz]

​PR

[Tim Graham <timograham@…>]

In 959d0c0:

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.

[Tim Graham <timograham@…>]

In e1bd944:

[2.2.x] 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.
Backport of 959d0c078a1c903cd1e4850932be77c4f0d2294d from master.