parameter 'widget' of BoundField.as_widget is ignored

[Sven R. Kunze]

'as_widget' has the parameter 'widget' which renders the field. However, this parameter is not used all the time during the 'as_widget' call.

The method 'data', which is (indirectly via 'value') called by 'as_widget', ALWAYS uses the default widget of the field which leads to issues like formatting etc.

We recognized this issue when using 'show_hidden_initial=True' and widgets for datetime objects.

The problem can also be observed when using BoundField.as_hidden as it uses 'as_widget' as well.

cf.
as_widget: ​https://github.com/django/django/blob/1.8a1/django/forms/forms.py#L566
data: ​https://github.com/django/django/blob/1.8a1/django/forms/forms.py#L609
as_hidden: ​https://github.com/django/django/blob/1.8a1/django/forms/forms.py#L602

Versions: 1.4, 1.5, 1.6, 1.7, 1.8, master

[Claude Paroz]

Replying to srkunze:

'as_widget' has the parameter 'widget' which renders the field. However, this parameter is not used all the time during the 'as_widget' call.

Sorry, but that's not true. The widget parameter is used in widget.render(...) at the end of the method. It's totally possible something is not working properly, but then we'll need a more thorough example to demonstrate the issue.

[Sven R. Kunze]

Replying to claudep:

The widget parameter is used in widget.render(...) at the end of the method.

I am not saying that this is the only possible place where the widget is used.

It's totally possible something is not working properly,

Please, carefully read my ticket again as I described what's wrong:

The method 'data', which is (indirectly via 'value') called by 'as_widget' ALWAYS used the default widget of the field which leads to issues like formatting etc.

but then we'll need a more thorough example to demonstrate the issue.

Cf. my ticket again:

The problem can also be observed when using BoundField.as_hidden as it uses 'as_widget' as well.

BouldField.as_hidden should always use the HiddenWidget during processing BoundField.as_widget, but BouldField.data uses the default widget which is not the HiddenWidget of course.

A more concise presentation of the issue:

`BoundField.as_hidden` -> `BoundField.as_widget` -> `BoundField.value` -> `BoundField.data`
(wants HiddenWidget)      (uses HiddenWidget)       (uses default widget) (uses default widget)

"->" means "uses"

BoundField.as_hidden specifies the HiddenWidget when it calls BoundField.as_widget
BoundField.value and BoundField.data do not respect the specified widget

Hence, the output is flawed.

[Tim Graham]

I'm also having trouble understanding your problem. It seems to me that BoundField.value/data need to use the field's widget rather than HiddenInput as the latter's value_from_datadict() doesn't know how to handle each field's data. Can you offer a patch for this issue?

[Sven R. Kunze]

It seems to me that BoundField.value/data need to use the field's widget rather than HiddenInput as the latter's value_from_datadict() doesn't know how to handle each field's data.

It should use the HiddenInput all the time during the as_hidden call.

Otherwise, the data given into a form (e.g. after a POST) and the data returned (e.g. when the form is rendered anew because of validation errors) differs.

Widgets set with hidden_initial=True render their data two times: with field.widget and with field.hidden_widget. Depending on whether or not its a bound form, results in different formatting of the value/data.

This would be an example fix to show what I mean: ​https://github.com/srkunze/django/commit/9480ce0c3853a47001bd12e339c263695c38978f

hidden_initial=True results in only_initial=True. That means the hidden_initial parts of the POST data should not be changed.

PS:
Could somebody explain me the difference between data and value with regards to BoundFields? Looks quite messy to me (e.g. BoundField.data returns value_from_datadict; prepare_value gets data, but it expects value according to its definition).

[Tim Graham]

Thanks for the details. Unfortunately your patch doesn't pass the test suite. Also, a new test showing the bug that's fixed is required. Will you continue working on it?

[Sven R. Kunze]

You are welcome.

Before continue working on it, I would like to discuss whether my patch makes sense and and I would like an answer on my PS.

Currently, it looks too messy to understand it properly; at least to me. So, I would like to clean it up before continuing.

[Tim Graham]

Could you write a test case so I can better understand the problem?

[Sven R. Kunze]

I give it a try soon.

In the meantime, I would appreciate it if you could elaborate on this:

Could somebody explain me the difference between data and value with regards to BoundFields? Looks quite messy to me (e.g. BoundField.data returns value_from_datadict; prepare_value gets data, but it expects value according to its definition).

[Tim Graham]

I think the docstrings explain the difference between data and value. I wouldn't get caught up in the fact that prepare_value() is called with a variable named data even though its parameter is named value.

[Sven R. Kunze]

I hope this illustrates our problem: ​https://github.com/srkunze/django/commit/d9fd705622a9bc948fcccc45f8363c182a9d748d

In case of an error of the form (thus, the form needs to be re-rendered), the hidden-initial should be re-rendered with the exact same value string as it was rendered in unbound state.

Each different state of the form (corresponding to each written test):

unbound
bound + changed
bound + unchanged

shows a different behaviour (fails, fails, succeeds) which is unfortunate for our internal testing library.

Replying to timgraham:

I think the docstrings explain the difference between data and value. I wouldn't get caught up in the fact that prepare_value() is called with a variable named data even though its parameter is named value.

Unfortunately, docstring explains data by saying "data" and value by saying "value". Thus, no relationship between data and value is defined clearly.

[Tim Graham]

I think datetimes maybe complicating the issue. It seems like at least one of the issues is that for the following form:

class MyForm(forms.Form):
    foo = forms.IntegerField(initial=1, show_hidden_initial=True)

form = MyForm({'foo': 5})

You expect the hidden input to be rendered with "1" instead of "5".

<label for="id_foo">Foo:</label>
<input id="id_foo" type="number" value="5" name="foo">
<input id="initial-id_foo" type="hidden" value="5" name="initial-foo">

Is that correct?

By the way, I tried looking at your tests, but they didn't pass even when I uncommented "# uncomment to see changes with correctly formatted hidden-initial" -- not sure if that was intended or not?

[Sven R. Kunze]

I think datetimes maybe complicating the issue.

Unfortunately, it is that complicated. Formatting (and its consistency over the life-time of the form) is the issue here. That is why I chose date inputs.

It seems like at least one of the issues is that for the following form:

class MyForm(forms.Form):
    foo = forms.IntegerField(initial=1, show_hidden_initial=True)

form = MyForm({'foo': 5})

Actually, it should look like this as browsers send initial-foo as well:

form = MyForm({'foo': 5, 'initial-foo': 1})

You expect the hidden input to be rendered with "1" instead of "5".

<label for="id_foo">Foo:</label>
<input id="id_foo" type="number" value="5" name="foo">
<input id="initial-id_foo" type="hidden" value="5" name="initial-foo">

Is that correct?

Interesting. You accidentally found another error.

I created another commit and added more tests to cover all that.

By the way, I tried looking at your tests, but they didn't pass even when I uncommented "# uncomment to see changes with correctly formatted hidden-initial" -- not sure if that was intended or not?

The comments were currently not intended to make the tests pass. They just represent how well-formed POST data would look like IF the unbound form would have produced correctly formatted hidden-initial data.

[Sven R. Kunze]

Here, we go: ​https://github.com/srkunze/django/commit/c1fdbeeb19a41b2f3dcc09f501bd5ff9298b846d

14 tests now:

D1) FAILED    test_boundfield__unbound_form__date
D2) FAILED    test_boundfield__changed_bound_form__date
D3) FAILED    test_boundfield__changed_bound_form__bogus_initial__date
D4) FAILED    test_boundfield__changed_bound_form__missing_initial__date
D5) SUCCEEDED test_boundfield__unchanged_bound_form__date
D6) SUCCEEDED test_boundfield__unchanged_bound_form__missing_initial__date

I1) SUCCEEDED test_boundfield__unbound_form__integer
I2) FAILED    test_boundfield__changed_bound_form__integer
I3) FAILED    test_boundfield__changed_bound_form__bogus_initial__integer
I4) FAILED    test_boundfield__changed_bound_form__missing_initial__integer
I5) SUCCEEDED test_boundfield__unchanged_bound_form__integer
I6) SUCCEEDED test_boundfield__unchanged_bound_form__missing_initial__integer

unbound_form - should be clear
changed_bound_form - form with data that has changed with regard to initial
unchanged_bound_form - form with data that has not changed with regard to initial
bogus_initial - user changed hidden initial
missing_initial - browser did not send hidden initial

[Tim Graham]

Well, I guess we can accept the ticket, but even after spending time looking at it, I still don't have a good understanding of the problem(s) or what a solution might look like.

[Sven R. Kunze]

Well, I guess we can accept the ticket, but even after spending time looking at it, I still don't have a good understanding of the problem(s)

I actually hoped you would be the one telling me. :D

or what a solution might look like.

After our conversation and looking at the test-cases, I am even unsure if my patch would touch the heart of the problem.

That is, btw., why I asked about the relationship between data and value. However, as it seems the problem lies even deeper.