[Django] #30153: inlines.js merge with wrong order in modeladmin
Django
-----------------------------------------+------------------------
Reporter: rayer4u | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30153>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: rayer4u | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rayer4u):
* Attachment "inlinetest.tgz" added.
This is the test program
Django
-------------------------------+--------------------------------------
Reporter: rayer4u | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:1>
Django
media in broken order
-------------------------------+------------------------------------
Reporter: rayer4u | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* cc: clincher, Johannes Hoppe (added)
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:2>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthias Kestenholz):
* cc: Matthias Kestenholz (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:3>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:4>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:5>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johannes Hoppe):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:6>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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'>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:7>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthias Kestenholz):
* Attachment "test.patch" added.
Test showing the problem
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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`)
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:8>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthias Kestenholz):
* Attachment "test.patch" added.
Test showing the problem
--
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthias Kestenholz):
* Attachment "test.2.patch" added.
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:9>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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)
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:10>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matthias Kestenholz):
[https://github.com/django/django/pull/10939 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:11>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthias Kestenholz):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:12>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"959d0c078a1c903cd1e4850932be77c4f0d2294d" 959d0c0]:
{{{
#!CommitTicketReference repository=""
revision="959d0c078a1c903cd1e4850932be77c4f0d2294d"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:13>
Django
media in broken order
-------------------------------+------------------------------------
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e1bd94496bcfe65c244bcd3ca668b900bf488326" e1bd944]:
{{{
#!CommitTicketReference repository=""
revision="e1bd94496bcfe65c244bcd3ca668b900bf488326"
[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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30153#comment:14>