Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#11632 closed (fixed)

show_hidden_initial=True produces invalid HTML

Reported by: geber@… Owned by: Karen Tracey
Component: Forms Version: 1.1
Severity: Keywords: show_hidden_initial
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When you have a model like this:

class MyModel(models.Model):
    datetime = models.TimestampField(_('Affected Time Start'), default=datetime.datetime.now)

And a Form like this:

class MyModelForm(forms.ModelForm):
    class Meta:
        model = MyModel
        fields = ('datetime',)

The generated HTML (in my case with prefix=2) looks like that:

<input type="text" name="2-datetime" value="2009-08-04 11:13:40" id="id_2-datetime" /><input type="hidden" name="initial-2-datetime" value="2009-08-04 11:13:40.694547" id="id_2-datetime" />

Obviously, the HTML ID is the same in the hidden field, which causes some JS-trouble.

Hope you'll fix it soon.

Cheers.

Attachments (1)

show_hidden_initial.diff (1.7 KB) - added by Mark Lavin 7 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 7 years ago by Michael Malone

Owner: changed from nobody to Michael Malone
Status: newassigned

comment:2 Changed 7 years ago by Michael Malone

Resolution: fixed
Status: assignedclosed
Triage Stage: UnreviewedAccepted

comment:3 Changed 7 years ago by Michael Malone

Resolution: fixed
Status: closedreopened

comment:4 Changed 7 years ago by Michael Malone

Owner: Michael Malone deleted
Status: reopenednew

comment:5 Changed 7 years ago by dc

Keywords: show_hidden_initial added
milestone: 1.2
Summary: default=datetime.datetime.now in ModelField creates dublicate HTML-IDs in ModelFormshow_hidden_initial=True produces invalid HTML
Version: 1.1SVN

This bug is valid for any field with show_hidden_initial=True

Another test case:

from django import forms
from django.db import models

class MyModel(models.Model):
    # callable default sets show_hidden_initial to True
    field1 = models.IntegerField(default=lambda: 0)
    class Meta:
        app_label = 'dummy'

class MyForm(forms.ModelForm):
    field2 = forms.CharField(max_length=50, show_hidden_initial=True)
    class Meta:
        model = MyModel

print MyForm()

Outputs duplicate ids:

<tr><th><label for="id_datetime">Datetime:</label></th><td><input type="text" name="datetime" value="1" id="id_datetime" /><input type="hidden" name="initial-datetime" value="1" id="id_datetime" /></td></tr>
<tr><th><label for="id_field2">Field2:</label></th><td><input id="id_field2" type="text" name="field2" maxlength="50" /><input type="hidden" name="initial-field2" id="id_field2" /></td></tr>

Looks like __unicode__() or as_hidden() in BoundField must be fixed.

comment:6 Changed 7 years ago by dc

Version: SVN1.1

Ok, i think better render id differently in as_widget() method if only_initial is True. Does we really need id for the hidden initial field?

comment:7 Changed 7 years ago by anonymous

I've just encountered this issue myself. I'm conflicted on what the solution should be. Since the <name> field gets 'initial-' prefixed to it, it seems like it would be consistent ti prefix the 'initial-' on the id. However, in practice that has issues. I have jquery code that finds dom objects by searching for them like this:

row.find('[id$=foo]')

ie, this finds a dom element whose name ends in 'foo'. Once I started using show_hidden_initial, my jquery code broke since it now finds two items whose name ends in 'foo'. I suspect other folks might do similar types of selectors. Adding 'initial-' to the front of the hidden initial input wouldn't help in this situation. Of course I could do a more compex jquery selector and ignore stuff that starts with initial-.

Getting rid of the id altogether on the hidden initial element fixes my issue, but leaves one in the unpleasant situation of never being able to find that hidden initial element in your dom - or at least not with a lot of trouble. So I don't really think it's a great solution. It seems like the most complete solution would allow the user to specify a "hidden initial auto id" when they instantiate a form, similar to the way they can specify the auto_id argument when they instantiate a form.

comment:8 in reply to:  7 Changed 7 years ago by Margie Roginski

Sorry, the above comment by "anonymous" is by me, forgot to log in before making it.

Changed 7 years ago by Mark Lavin

Attachment: show_hidden_initial.diff added

comment:9 Changed 7 years ago by Mark Lavin

Has patch: set

While it's a shame that it doesn't help your jQuery selector I think using the 'initial-' prefix is a consistent with how the field names are generated. I added a patch that generates prefix to the id on the hidden initial field. I think as long as it is clear and consistent how the names and ids will be generated people can make the rest of their project work as far as selecting the elements and not generating conflicting names/ids.

comment:10 Changed 7 years ago by anonymous

Owner: set to Karen Tracey

comment:11 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: newclosed

(In [11826]) Fixed #11632: Fixed the id for hidden initial widget so that it is different from the id for its visible counterpart. Thanks geber@… and Mark Lavin.

comment:12 Changed 7 years ago by Karen Tracey

(In [11827]) [1.1.X] Fixed #11632: Fixed the id for hidden initial widget so that it is different from the id for its visible counterpart. Thanks geber@… and Mark Lavin.

r11826 from trunk.

comment:13 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: closedreopened

Reopening to fix broken test resulting from this fix.

comment:14 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: reopenedclosed

(In [11835]) Fixed #11632: Fixed a test broken by r11826 that relied on the exact (invalid HTML) id of an initial hidden input field.

comment:15 Changed 7 years ago by Karen Tracey

(In [11836]) [1.1.X] Fixed #11632: Fixed a test broken by r11827 that relied on the exact (invalid HTML) id of an initial hidden input field.

comment:16 in reply to:  9 Changed 7 years ago by Margie Roginski

Replying to mlavin:

While it's a shame that it doesn't help your jQuery selector I think using the 'initial-' prefix is a consistent with how the field names are generated. I added a patch that generates prefix to the id on the hidden initial field. I think as long as it is clear and consistent how the names and ids will be generated people can make the rest of their project work as far as selecting the elements and not generating conflicting names/ids.

Thanks for putting the patch in, Mark. I agree that prefixing with initial- is a good and consistent fix.

comment:17 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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