Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4174 closed (fixed)

[newforms-admin] prepopulated_fields results in javascript error (None is not defined)

Reported by: forest@… Owned by: adrian
Component: contrib.admin Version: newforms-admin
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by adrian)

I get this in Firefox 2:

Error: None is not defined
Source File: http://localhost:8080/admin/page/section/add/
Line: 142

The javascript source is as follows:

<script type="text/javascript">

    document.getElementById("id_slug").onchange = function() { this._changed = true; };
    document.getElementById("id_title").onkeyup = function() {
        var e = document.getElementById("id_slug");
        if (!e._changed) { e.value = URLify(document.getElementById("id_title").value, None); }



Attachments (1)

4174_newforms_admin_prepopulated_from.diff (570 bytes) - added by Brian Rosner <brosner@…> 8 years ago.
patch to fix #4174

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by Simon G. <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from other branch to newforms branch

comment:2 Changed 8 years ago by Simon G. <dev@…>

  • Summary changed from prepopulated_fields results in javascript error (newforms-admin) to [newforms-admin] prepopulated_fields results in javascript error (None is not defined)

comment:3 Changed 8 years ago by adrian

  • Version changed from newforms branch to newforms-admin

comment:4 Changed 8 years ago by Brian Rosner <brosner@…>

I came across this problem as well. I spent a little bit of time looking through the source to find the root of this problem.

In django/contrib/admin/templates/admin/change_form.html the line that causes the problem is (line 87 as of r5499):

if (!e._changed) { e.value = URLify({% for innerdep in field.dependencies %}document.getElementById("{{ innerdep.auto_id }}").value{% if not forloop.last %} + ' ' + {% endif %}{% endfor %}, {{ field.field.field.max_length }}); }

The call to {{ field.field.field.max_length }} results in None. This is due to maxlength from the model not being push through to the formfield created by the model Field class. This may also be be field independent, but since pre_populated uses CharFields, it would be more specific to that.

I guess the real question is, should maxlength be pushed over to the form field? I would it should, but perhaps some people will confuse that with an enforcement of that number in the actual form field. I don't know. ;) If it is as simple to move it over, I'd be up for writing a patch to fix that.

comment:5 Changed 8 years ago by Brian Rosner <brosner@…>

I stand corrected. My comments above should really be disregraded. I looked at this a little more in depth and it turns out this is a problem with the SlugField field implmentation. It subclasses Field and doesn't have a formfield method which Field's formfield method does not push over maxlength. If you subclass SlugField with CharField the problem is solved.

Changed 8 years ago by Brian Rosner <brosner@…>

patch to fix #4174

comment:6 Changed 8 years ago by anonymous

Works fine for me, thanks for the patch.

comment:7 Changed 8 years ago by Brian Rosner <brosner@…>

I believe this has been fixed as of r5926 in newforms-admin. Russell can you comment on this? If so, this ticket can be closed.

comment:8 Changed 8 years ago by Brian Rosner <brosner@…>

Err, ignore my comment above, this is the wrong ticket ;)

comment:9 Changed 8 years ago by nirvdrum

I can also verify that the patch does work.

comment:10 Changed 8 years ago by adrian

  • Description modified (diff)

Fixed formatting in description.

comment:11 Changed 8 years ago by jkocherhans

  • Resolution set to fixed
  • Status changed from new to closed

Doh! Sorry, I fixed this earlier tonight, completely forgot to look at the discussion here, and came to the same conclusions. That would have saved a few minutes of my life :) Fixed in [6056]

comment:12 Changed 8 years ago by Brian Rosner <brosner@…>

Hey, thanks for the credit ;) It looks like this ticket should be a duplicate of #3557. r6065 looks like to solve both.

Note: See TracTickets for help on using tickets.
Back to Top