[patch] Admin js option does not honor absolute urls

[eugene@…]

Previously js option produced script blocks using verbatim path. Example:

class Document(meta.Model):
    # fields
    class META:
        admin = meta.Admin(
            js = (
                '/tinymce/jscripts/tiny_mce/tiny_mce_gzip.php',
            ),
        )

produced:

<script type="text/javascript" src="/tinymce/jscripts/tiny_mce/tiny_mce_gzip.php"></script>

New admin produces different string:

<script type="text/javascript" src="/media//tinymce/jscripts/tiny_mce/tiny_mce_gzip.php"></script>

Essentially it is the same js path but it is now prefixed by /media/. It is easy to correct in my case: change symlink, and remove leading '/' in js.

Anyway we have a backward-incompatible difference. We have to decide how to handle it: old way or new way. The final decision should be documented.

New way is not bad. You can argue that it reduces number of entries in .htaccess or whatever non-Django mechanism is used for file serving. But it forces to change existing model files, which use extra JavaScript for Admin interface.

[edgars@…]

+1

[Joeboy]

I think the new behaviour is really quite irritating. If I want to use a single js file that varies from site to site (eg. a tinymce config file) I have to make a copy of the whole admin media folder for each site, whereas previously I could just stick it in /media. Am I missing something or is this just gratuitously annoying?

I'd have thought the sensible system would be to not prefix if the url begins with a slash.

[gandalf@…]

Here is a patch that modifies this behaviour in two ways:

a) It checks for / at the beginning of the lines, and if it finds it, it doesn't include ADMIN_MEDIA_URL before it
b) It checks with regular expression if it is url (starting with http, ftp or https) and also doesn't include ADMIN_MEDIA_URL in this case.

otherwise, it inserts ADMIN_MEDIA_URL at the beginning

[gandalf@…]

add checking for / and url's for js in admin media

[gandalf@…]

updated patch for magic-removal

[gandalf@…]

Any comment on why this patch is not acceptable for inclusion into svn?

[xale]

It's been six months since this ticked was created and the patch still isn't in trunk as far as I can see. Is there any readon why it's not commited yet?

[xale]

I've looked at the patch and it's pretty horible.

What we really need here is either remove this prefix at all and change all admin code to use full path or change templates and template tags to include user defined JS URLs without prefix.

[nesh <nesh [at] studioquattro [dot] co [dot] yu>]

What is the status of this patch? I really need this because I don't put admin media within my media folder and I hate to use patched django source.

I'm definitely against prepending ADMIN_MEDIA_PREFIX to custom JS.

IMHO this patch is OK.

Just a suggestion, remove regexp from the function like this (small optimization).

_URL_PATTERN = re.compile(r'''(?x)((http|https|ftp)://(\w+[:.]?){2,}(/?|[^ \n\r"']+[\w/!?.=#])(?=[\s\.,>)"'\]]))''')
def include_admin_script(script_path):
   if _URL_PATTERN.match(script_path) or script_path[0] == '/':
       return '<script type="text/javascript" src="%s"></script>' % (script_path)
   else:
       return '<script type="text/javascript" src="%s%s"></script>' % (settings.ADMIN_MEDIA_PREFIX, script_path)
include_admin_script = register.simple_tag(include_admin_script)

[nesh <nesh [at] studioquattro [dot] co [dot] yu>]

diff against [3411]

[mhf@…]

This problem can be solved with urlparse.urljoin:

>>> from urlparse import urljoin
>>> urljoin('/media', 'somescript.js')
'/somescript.js'
>>> urljoin('/media', '/anotherpath/somescript.js')
'/anotherpath/somescript.js'
>>> urljoin('/media', 'http://example.org/somescript.js')
'http://example.org/somescript.js'

[mhf@…]

Patch (against r4185), which uses urljoin

[mhf@…]

Attached a patch against r4185, which uses urljoin instead of a regular expression.

Should there be a regression test for this?

[Chris Beaven]

Thanks mhf, I'll let the core decide if we need a regression test and mark as ready for the moment.

[Adrian Holovaty]

Marking this as fixed because, as of [4382] in the newforms-admin branch, you can implement the javascript() method on your class Admin to specify arbitrary JavaScript.