Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26949 closed Bug (fixed)

Disabled forms.JSONField crashes with TypeError

Reported by: Tim Graham Owned by: Olexander Yermakov
Component: contrib.postgres Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The fix for #25532 is partially reverted to fix the regression in #26917 but this means the following test fails with TypeError: the JSON object must be str, not 'list'.:

def test_formfield_disabled(self):
    class JsonForm(Form):
        name = CharField()
        jfield = forms.JSONField(disabled=True)

    form = JsonForm({'name': 'xyz', 'jfield': '["bar"]'}, initial={'jfield': ['foo']})
    self.assertIn('[&quot;foo&quot;]</textarea>', form.as_p())

Attachments (1)

0001-Fix-disabled-forms.JSONField-crash-with-TypeError.patch​ (810 bytes ) - added by Olexander Yermakov 8 years ago.
As I see the problem with previous fix was because assumption that initial values are already cleaned is wrong. I added these lines to omit json.loads() for disable field. But I think that's the obvious fix, means there are downsides here if it's not been done before, so I'm not sure about it. Could you, please, advice what could be wrong with this approach? Thanks!

Download all attachments as: .zip

Change History (10)

comment:1 by Olexander Yermakov, 8 years ago

Owner: set to Olexander Yermakov
Status: new → assigned

by Olexander Yermakov, 8 years ago

Attachment: 0001-Fix-disabled-forms.JSONField-crash-with-TypeError.patch​ added

As I see the problem with previous fix was because assumption that initial values are already cleaned is wrong. I added these lines to omit json.loads() for disable field. But I think that's the obvious fix, means there are downsides here if it's not been done before, so I'm not sure about it. Could you, please, advice what could be wrong with this approach? Thanks!

comment:2 by Tim Graham, 8 years ago

Has patch: set

Looks okay to me. I added that fix to my ​PR for the other ticket. Thanks!

comment:3 by Olexander Yermakov, 8 years ago

Should I create PR for the 1.9 branch or it's not required?

comment:4 by Tim Graham, 8 years ago

Not needed since the committer will backport any commits as needed. I think I'll just backport to 1.10 only since 1.9.x will be in security fix only mode after the 1.10 release next week.

comment:5 by Olexander Yermakov, 8 years ago

Ok. Thanks for the fast and clear response!

comment:6 by Tim Graham <timograham@…>, 8 years ago

In a5f85d89:

Fixed #26917 -- Fixed crash in disabled ModelChoiceFields.

Partially reverted refs #25532 to fix a regression in Django 1.10.
This reintroduces a crash for disabled forms.JSONField (refs #26949),
however, that issue is also present on Django 1.9.

Thanks Ryan Schave for the test.

comment:7 by Tim Graham <timograham@…>, 8 years ago

Resolution: → fixed
Status: assigned → closed

In 68de48c:

Fixed #26949 -- Fixed crash of disabled forms.JSONField.

comment:8 by Tim Graham <timograham@…>, 8 years ago

In 3744fc1:

[1.10.x] Fixed #26917 -- Fixed crash in disabled ModelChoiceFields.

Partially reverted refs #25532 to fix a regression in Django 1.10.
This reintroduces a crash for disabled forms.JSONField (refs #26949),
however, that issue is also present on Django 1.9.

Thanks Ryan Schave for the test.

Backport of a5f85d891b51d7ceb4f9e422e3e4f5c741062288 from master

comment:9 by Tim Graham <timograham@…>, 8 years ago

In 714e287d:

[1.10.x] Fixed #26949 -- Fixed crash of disabled forms.JSONField.

Backport of 68de48c96328e13d5dbdb1f3006e4a1ca74f3c34 from master

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