#11967 closed (fixed)
[patch] Firefox 3.5/TinyMCE collision breaks DateTimeShortcuts.js in admin
| Reported by: | Daniel Roseman | Owned by: | Jacob |
|---|---|---|---|
| Component: | contrib.admin | Version: | 1.1 |
| Severity: | Keywords: | ||
| Cc: | smulloni@…, olau@…, justin@… | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The combination of Firefox 3.5 with TinyMCE on an admin add/edit screen which contains a date or time field causes the calendar/clock icons not to appear, and spurious 404/500 errors to appear in logs.
I have reproduced this on Mac and Linux (Ubuntu), and more reports are here:
http://groups.google.com/group/django-users/browse_thread/thread/14beb2d0cab45257
http://groups.google.com/group/django-users/browse_thread/thread/be50deac49247f80
The cause appears to be the way DateTimeShortcuts.js works out the value of admin_media_prefix, by searching for its own name in the list of scripts returned by document.getElementsByTagName('script'). TinyMCE dynamically injects various scripts into the page, and some issue with Firefox 3.5 causes the it to no longer return the full list of scripts in the DOM. Therefore, DateTimeShortcuts.js is not found, and the script incorrectly uses a blank value as the media prefix, leading to it requesting the icons via a relative path from the current page - eg /admin/app/model/1234/img/admin/icon_calendar.gif - which causes a ValueError: invalid literal for int() with base 10 in Django.
Although this is clearly a bug in Firefox and/or TinyMCE, I think it's worth fixing in Django anyway as the way DateTimeShortcuts works out admin_media_prefix is quite crufty, and using TinyMCE in the admin is fairly popular. The proposed patch simply defines a global admin_media_prefix variable in the change_form template, and DateTimeShortcuts.js uses that if defined (it still falls back to the old method otherwise). Having admin_media_prefix available to all scripts would seem like a good idea in any case.
Attachments (2)
Change History (11)
by , 15 years ago
| Attachment: | admin_media.diff added |
|---|
comment:1 by , 15 years ago
| Cc: | added |
|---|
comment:2 by , 15 years ago
comment:3 by , 15 years ago
| milestone: | → 1.2 |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
comment:4 by , 15 years ago
There was a bug in that patch. admin_media_prefix is used as a variable with {{ admin_media_prefix }} instead of as a tag {% admin_media_prefix %}. Here's a fixed patch.
comment:5 by , 15 years ago
| Cc: | added |
|---|
comment:6 by , 15 years ago
| Cc: | added |
|---|
comment:7 by , 15 years ago
I think it is much better to not fall back to the current method - a single mechanism that works completely or not at all is far preferable to a situation where you can forget part of the solution and it still works most of the time.
However, the single mechanism needs to be robust, and it isn't yet - every time you use DateTimeShortcuts.js you have to remember to add that var definition to the template. How about just putting that var definition in 'admin/base.html'?
comment:8 by , 15 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
(In [13002]) Fixed #11967: use a different technique to get ADMIN_MEDIA_PREFIX in admin JS.
We now store ADMIN_MEDIA_PREFIX on the window object and let JS files read it
from there. The old technique -- looking a <script> tags and trying to deduce
the prefix -- broke with libraries like TineMCE that dynamically add <script>
tags to the <head>.
This is potentially backwards-incompatible for folks who've overridden
admin/base.html; they'll need to add the proper <script> line to
their new admin/base.html. For that reason, this changeset shouldn't
be backported to 1.1.X.
This is driving me nuts too. It wouldn't be so bad if this was a 404, but combined with #11191 and #3785 it generates a ton of 500 errors. :/
Any chance this will be in 1.0.5 or 1.1.2?