Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#11632 closed (fixed)

show_hidden_initial=True produces invalid HTML

Reported by: geber@… Owned by: kmtracey
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 mlavin 5 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 6 years ago by mmalone

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to mmalone
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 6 years ago by mmalone

  • Resolution set to fixed
  • Status changed from assigned to closed
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 6 years ago by mmalone

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:4 Changed 6 years ago by mmalone

  • Owner mmalone deleted
  • Status changed from reopened to new

comment:5 Changed 6 years ago by dc

  • Keywords show_hidden_initial added
  • milestone set to 1.2
  • Summary changed from default=datetime.datetime.now in ModelField creates dublicate HTML-IDs in ModelForm to show_hidden_initial=True produces invalid HTML
  • Version changed from 1.1 to SVN

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 6 years ago by dc

  • Version changed from SVN to 1.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 follow-up: Changed 5 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 5 years ago by margieroginski

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

Changed 5 years ago by mlavin

comment:9 follow-up: Changed 5 years ago by mlavin

  • 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 5 years ago by anonymous

  • Owner set to kmtracey

comment:11 Changed 5 years ago by kmtracey

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

(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 5 years ago by kmtracey

(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 5 years ago by kmtracey

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to fix broken test resulting from this fix.

comment:14 Changed 5 years ago by kmtracey

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

(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 5 years ago by kmtracey

(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 5 years ago by margieroginski

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 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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