Opened 9 years ago

Closed 8 years ago

#24871 closed Bug (wontfix)

Textarea widget has redundant \r\n when writing XHTML

Reported by: Tomáš Pecina Owned by: nobody
Component: Forms Version: 1.8
Severity: Normal Keywords: textarea, widget
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The Textarea widget contains leading \r\n, which is wrong in certain contexts (I experienced the problem with one of my Django projects outputting XML-serialized HTML5, which copied the newline to initial field value). These characters are unneeded and could and should safely be deleted. The patch (for Django 1.6, but there's no difference in later revisions) is attached.

Attachments (3)

widgets.patch (584 bytes ) - added by Tomáš Pecina 9 years ago.
textarea1.html (129 bytes ) - added by Tomáš Pecina 9 years ago.
textarea1.xhtml (221 bytes ) - added by Tomáš Pecina 9 years ago.

Download all attachments as: .zip

Change History (15)

by Tomáš Pecina, 9 years ago

Attachment: widgets.patch added

comment:1 by Tim Graham, 9 years ago

Those characters are to fix #8627. Has the situation with how browsers handle that situation changed or do we have two different use cases such that we have to make a decision which one to support?

comment:2 by Tim Graham, 9 years ago

The test added in #8627 does fail in Firefox if the proposed patch is applied.

by Tomáš Pecina, 9 years ago

Attachment: textarea1.html added

by Tomáš Pecina, 9 years ago

Attachment: textarea1.xhtml added

comment:3 by Tomáš Pecina, 9 years ago

I added two more files, simple test forms. Their contents is practically identical, except that one is .html and the other .xhtml. Both Chrome and FF display them differently.

Thus, in my opinion, #8627 did not take into account the differences in behavior of browsers in XHTML mode, and the original patch should be reviewed. A clumsy, though practicable, workaround is to add the extra newline only if the content generated is (plain) HTML.

comment:4 by Tim Graham, 9 years ago

For what it's worth, Rails also adds a leading newline (issue).

I don't see a good way of making the newline conditional on whether the widget is being rendered as HTML or XHTML as the widget doesn't have knowledge of that.

comment:5 by Tomáš Pecina, 9 years ago

That's right, so the information weather to add a newline would have to be supplied by the developer somehow, eg, in a settings variable.

To sum it up, standard HTML requires a leading newline as all browsers are required to ignore it, by the HTML standard. Stripping it would break a lot of existing code because leading newlines cannot be safely stripped, as is the case with CMS's, etc.

On the other hand, when Django is outputting XML-serialized code, there is no way to get rid of the extra newline, except using a custom Textarea widget.

I see no easy way out, the "lesser evil" appears to be a new settings variable.

comment:6 by Tim Graham, 9 years ago

The problem with a setting is that it forces a project to choose XHTML or HTML (can't use both together). contrib.admin, for example, requires HTML (#23908).

comment:7 by Marc Tamlyn, 9 years ago

Summary: Textarea widget has redundant \r\nTextarea widget has redundant \r\n when writing XHTML

New settings are definitely not the lesser of the evils!

I'm severely unconvinced we should be careful about support for XHTML5. IE doesn't support it, and it seems to be a much more non-standard practice than it was in html4 days. I would suggest maybe a documentation note with the text area widget, but I'm borderline to just closing this.

comment:8 by Tomáš Pecina, 9 years ago

If I had a time machine, I could easily find out if the XML-serialized HTML (aka "XHTML5") has any future. But I don't have one and at the moment, there are people - including me - using the XHTML format, and these are made to create custom textarea widgets. The primary problem is the inconsistent behavior of browsers (due to a failure of HTML/XML authors to deal with the whitespace trouble more elegantly in the first place). Therefore, it might be a good idea for Django to address this issue, by way of a settings variable or a widget parameter (or both). Well, if XHTML is dead, let's forget about it and close the ticket as obsolete and antiquated.

comment:9 by Carl Meyer, 9 years ago

Triage Stage: UnreviewedAccepted

I think a setting to control this is a non-starter, and we can drop it from consideration. No other aspect of form widget rendering (or really, form behavior in general) is configured via settings. Not to mention Tim's objection, which is that a project wide setting fails to account for a project which may need both behaviors (eg because it is mostly XHTML5 but uses the admin).

Creating a custom widget subclass is not onerous. I do it all the time, for all sorts of reasons. My inclination is that that's an adequate solution for this case.

That said, I wouldn't have any problem with a prepend_newline parameter to Textarea which defaults to True. The justification for this is not so much XHTML5 specifically, and more just that the automatic new line stripping is conceptually odd behavior to begin with, added by browsers to workaround careless markup, and it seems reasonable to have a way to avoid our workaround for that workaround and revert to what would otherwise be the straightforward intuitive behavior for the Textarea widget (that is, leaving the provided contents alone).

I do think the added new line behavior is something that ought to be documented regardless. Accepting this ticket on the basis that at least docs should be added.

Last edited 9 years ago by Carl Meyer (previous) (diff)

comment:10 by Tomáš Pecina, 9 years ago

The problem is not with having to subclass the widget (I never use non-custom widgets in my projects anyway) but that there is currently no elegant way of doing it; as a tentative and provisional fix, I'm overriding the render method with taking whatever the superclass' render returns and replacing the newline after the first ">" with an empty string, like this:

    def render(self, *args, **kwargs):
        return mark_safe(super(taw, self).render(*args, **kwargs).replace('>\r\n', '>', 1))

Having a widget parameter would definitely be nice.

Last edited 9 years ago by Tomáš Pecina (previous) (diff)

comment:11 by Carl Meyer, 9 years ago

Yes, good point - I can see that in this case there's no elegant way for a subclass to change this behavior.

A more general solution would be to move the HTML template into a class attribute that a subclass could override (or one of these days finally fix #15667).

On further thought, I think I'm more inclined towards the class attribute than the parameter. The goal is to eventually move towards #15667, and once we arrive in that world (where all widget markup is overrideable via the template system) the existence of such a specific parameter for changing one aspect of rendering a Textarea would seem a bit of a wart, I think. Whereas moving the HTML template of a widget to a class attribute is consistent with other existing widgets, and offers a flexibility that's more similar to that of template-based widget rendering.

Is anyone opposed to moving the HTML template for Textarea to a class attribute of the widget?

comment:12 by Tim Graham, 8 years ago

Resolution: wontfix
Status: newclosed

Closing since #15667 is close to merge.

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