6776 mark_safe breaks newforms_admin

[Tom Vergote]

In the latest changeset [6776] on newforms-admin breaks the admin interface

some return's are merged to use the mark_safe method, but the imports were not adapted/merged

see patch

[Petr Marhoun <petr.marhoun@…>]

I have more problems - see the patch.

I don't understand autoescape fully so I am not sure with solutions. But problems are real.

[Malcolm Tredinnick]

Comment 3 is a dupe of #6059. Let's keep one issue per ticket, please.

[Karen Tracey <kmtracey@…>]

Yes, the change to django/contrib/admin/templates/admin/change_form.html is an alternative fix for #6059. I took the approach of using mark_safe on the variable before sending it to the template instead of using the safe filter in template...got the impression from other code this was a preferred way to handle it?

The changes to django/contrib/admin/widgets.py are similarly covered by #6060, though the #6060 patch also attempts to mark_safe the output of the AdminSplitDateTime widget, so there's a slight difference there.

I haven't yet run into the problems requiring the changes to:

django/contrib/admin/templates/admin/change_list_results.html

django/contrib/admin/templates/admin/includes/fieldset.html

django/newforms/widgets.py

so they might be needing their own tickets, since Malcolm indicates we should stick to one issue per ticket. Though I also see how this could be viewed as all one big issue -- autoescape and newforms-admin...

[Petr Marhoun <petr.marhoun@…>]

Replying to Karen Tracey <kmtracey@gmail.com>:

The changes to django/contrib/admin/widgets.py are similarly covered by #6060, though the #6060 patch also attempts to mark_safe the output of the AdminSplitDateTime widget, so there's a slight difference there.

AdminSplitDateTimeWidget inherits from SplitDateTimeWidget and SplitDateTimeWidgets inherits from MultiWidget. I don't know which place is best for fixing - I have decided for MultiWidget.

I haven't yet run into the problems requiring the changes to:

django/contrib/admin/templates/admin/change_list_results.html

Maybe you haven't saw them only - could you check style for sorting? I sometimes use application/xhtml+xml and then I see (without patch):

XML Parsing Error: not well-formed
Location: http://hradec:8000/admin/auth/user/
Line Number 129, Column 11:<th class=&quot;sorted ascending&quot;>
----------^

django/contrib/admin/templates/admin/includes/fieldset.html

It is for validation errors.

django/newforms/widgets.py

It is the MultiWidget.

so they might be needing their own tickets, since Malcolm indicates we should stick to one issue per ticket. Though I also see how this could be viewed as all one big issue -- autoescape and newforms-admin...

Yes, it was my idea (one big issue) and the time difference was only a few minutes - when I started to read my patches before uploading, #6059 and #6060 were not here. But I am afraid I have no time today for new tickets and patches.

[Karen Tracey <kmtracey@…>]

Replying to Petr Marhoun <petr.marhoun@gmail.com>:

Replying to Karen Tracey <kmtracey@gmail.com>:

The changes to django/contrib/admin/widgets.py are similarly covered by #6060, though the #6060 patch also attempts to mark_safe the output of the AdminSplitDateTime widget, so there's a slight difference there.

AdminSplitDateTimeWidget inherits from SplitDateTimeWidget and SplitDateTimeWidgets inherits from MultiWidget. I don't know which place is best for fixing - I have decided for MultiWidget.

Ah, yes, that's probably a better place to do it.

I haven't yet run into the problems requiring the changes to:

django/contrib/admin/templates/admin/change_list_results.html

Maybe you haven't saw them only - could you check style for sorting? I sometimes use application/xhtml+xml and then I see (without patch):

XML Parsing Error: not well-formed
Location: http://hradec:8000/admin/auth/user/
Line Number 129, Column 11:<th class=&quot;sorted ascending&quot;>
----------^

OK, I see the escaping in the source, it just wasn't causing a visible problem in my environment/browser. A better (in that I don't see much use of the safe filter in the admin templates, but rather mark_safe used in the code) place to fix it may be in templatetags/admin_list.py, where the variable is set.

django/contrib/admin/templates/admin/includes/fieldset.html

It is for validation errors.

Hadn't gotten to validating input earlier. This one can be fixed at the source in options.py by calling mark_safe on the return value of errors for FieldLine.

django/newforms/widgets.py

It is the MultiWidget.

Gotcha.

so they might be needing their own tickets, since Malcolm indicates we should stick to one issue per ticket. Though I also see how this could be viewed as all one big issue -- autoescape and newforms-admin...

Yes, it was my idea (one big issue) and the time difference was only a few minutes - when I started to read my patches before uploading, #6059 and #6060 were not here. But I am afraid I have no time today for new tickets and patches.

Yeah, we were running into the same problems simultaneously (though I had seen your fix for the missing import when I started). I too am running out of time to spend on this today, so I am going to attach one last diff to this ticket, that contains all the fixes I am using. Similar to the 2nd patch except I used mark_safe at the source rather than the safe filter in the templates. I see Joseph is starting to commit fixes anyway so shortly a new checkout will probably resolve all of these.

[Tom Vergote]

I agree, there is a lot more to this, I just patched what i noticed this morning, and then was busy fixing some of my own code that broke after the changeset.

[jkocherhans]

I'll be working on tests shortly based on Karen's last patch. tvrg, if you are planning on writing the tests, and expect you'll be able to finish in the next few hours, please reassign yourself to this ticket.

[Tom Vergote]

I won't finish any tests today (considering day and timezone :))

[jkocherhans]

Doh. This was fixed in [6782], but I put the wrong ticket number in the commit message.