[Form] A form which is an empty collection has no CSRF token #4000
Comments
|
@fabpot can you close this one ? it reference to a PR who was merged |
|
@fabpot can you reopen this one ? It references a PR that causes the trouble |
|
I can reopen. |
|
sorry @vicb for the misunderstood. |
fabpot
added a commit
that referenced
this issue
Apr 27, 2012
Commits ------- 246c885 [Form] Fixed: Default value of 'error_bubbling' is now determined by the 'single_control' option d3bb4d0 [Form] Renamed option 'primitive' to 'single_control' 167e64f [Form] Fixed: Field attributes are not rendered in the label anymore. Label attributes are now passed in "label_attr" 68018a1 [Form] Dropped useless test that is guaranteed by OptionsParser tests and that needs to be adapted very often 649752c [Form] Fixed: CSRF token was not displayed on empty complex forms c623fcf [Form] Fixed: CSRF protection did not run if token was missing eb75ab1 [Form] Fixed results of the FieldType+FormType merge. Discussion ---------- [Form] Fixed errors introduced in the FieldType+FormType merge Bug fix: yes Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes Fixes the following tickets: #3994, #4000, #2294, #4118 Todo: -  --------------------------------------------------------------------------- by Tobion at 2012-04-22T15:39:20Z `primitive` is a pretty abstract option name. It depends on the person what he considers primitive. Maybe more explicit naming or better documentation what it means. --------------------------------------------------------------------------- by bschussek at 2012-04-22T15:47:29Z Better suggestions? The distinction here is between primitive and complex forms, where primitive forms are such forms that can be represented by a single HTML tag. This obviously needs to be documented. --------------------------------------------------------------------------- by Tobion at 2012-04-22T15:49:45Z Maybe `single_widget` or something like that. --------------------------------------------------------------------------- by vicb at 2012-04-23T13:09:43Z @Tobion @bschussek would `elementary` be better than `primitive` ? --------------------------------------------------------------------------- by vicb at 2012-04-23T13:17:04Z and `compound \ composite` better than `complex` ? --------------------------------------------------------------------------- by bschussek at 2012-04-23T14:08:33Z @vicb I fail to see how elementary/compound is easier to understand than primitive/complex. Maybe single_widget, but what's the opposite of this case? multi_widget? --------------------------------------------------------------------------- by vicb at 2012-04-23T14:15:09Z Actually I am fine with anything... as long as it is documented. --------------------------------------------------------------------------- by bschussek at 2012-04-23T14:22:31Z Still I think that this unveals a more profound naming problem. How do we (also in the documentation) name forms with children (formerly "forms") and forms without children (formerly "fields")? Should we refer to them as * forms and fields? * complex and primitive forms? * ... We must first answer this question before we can find an intuitive option name. If the documentation always switches between different terminologies, neither will it be understandable nor will this option be easy to remember. --------------------------------------------------------------------------- by vicb at 2012-04-23T15:10:32Z > Still I think that this unveals a more profound naming problem. How do we (also in the documentation) name forms with children (formerly "forms") and forms without children (formerly "fields")? To make it clear, I would rather say forms that **can have** children and forms that **can not have** children (i.e. Empty collections have no children but they can have and this is reason why you have to introduce those options, right ? - that could be a good example for the doc). It will probably be better to refer to "complex" / "primitive" forms in the doc (and use the "form" / "field" terms to explain them). Note: I think @Tobion concern is that "primitive" / "complex" could be pejorative terms (this is why I have proposed "elementary" / "compound"). --------------------------------------------------------------------------- by Tobion at 2012-04-23T16:00:54Z 1. primitive/complex is subjective (and could be pejorative too) 2. elementary/compound is more explicit so probably better than primitive/complex 3. I dislike this option in general. Does it make sense to change this option from a user perspective? I guess it's always the same as long as the widget structure stays the same. So it should be resolved at a higher level dynamically from the widget structure and not exposed to any configuration. 4. In documentation I would use the terms forms and fields. Because all people with HTML knowledge will understand that fields cannot have sub-fields whereas forms can. But since this distinction is not findable in code, it should be mentioned that all these are implemented as a form hierarchy. --------------------------------------------------------------------------- by mvrhov at 2012-04-23T16:02:00Z how about simple and complex? --------------------------------------------------------------------------- by bschussek at 2012-04-23T16:06:33Z @Tobion It does not make sense to change this option from the user perspective, still the overloading type has to propagate to FormType whether it is a form or a field, so that the default behaviour is correct. A second option how to implement this is to add a method `isField` to FormTypeInterface that can be overloaded and receives the options. I don't really like to introduce new methods here unless absolutely required. What about renaming the option "primitive" to "is_field"? The blocks in the template would then be named "form_widget_field" and "form_widget_form". --------------------------------------------------------------------------- by tristanbes at 2012-04-25T14:01:06Z Oh, I should've seen this before, i thought I was doing something wrong. (empty collections gets an input field bug) Please big :UP: on this. When will it be merged ? @bschussek --------------------------------------------------------------------------- by Tobion at 2012-04-25T15:30:28Z +1 for "is_field" and "form_widget_field" but I would rather use "form_widget_compound" instead of "form_widget_form" which is quite strange. --------------------------------------------------------------------------- by bschussek at 2012-04-26T16:34:04Z @Tobion "simple" and "compound" then? --------------------------------------------------------------------------- by Tobion at 2012-04-26T16:49:58Z no "field" and "compound" --------------------------------------------------------------------------- by bschussek at 2012-04-26T17:17:02Z I don't like "field" for a simple reason: Consider the "date" type. We are typically speaking of the "date" field there. But technically, the "date" field is a compound field. So? --------------------------------------------------------------------------- by Tobion at 2012-04-26T21:17:37Z I don't understand the open question. You proposed "is_field" and "form_widget_field" yourself. So calling the template block "form_widget_field" is a comprehensible consequence of "is_field". I wouldn't call the date type with multiple inputs a field. --------------------------------------------------------------------------- by tristanbes at 2012-04-26T21:52:39Z We should take a decision cause right here i got all my forms that are broken because of the empty collection rendering as input field :-). I guess we are many in that situation. --------------------------------------------------------------------------- by bschussek at 2012-04-27T08:28:16Z I renamed "primitive" to "single_control" now to match with the HTML specification which names all input elements (input, select etc.) "controls". The opposite is now "compound". Meanwhile, I added a fix for #4118. @fabpot This is ready for merge now. --------------------------------------------------------------------------- by Tobion at 2012-04-27T10:22:49Z Hm, I know naming things is hard and sometimes not really important. But since users need to know which block to override, it is essential to make it clear. I think there is still one issue. The block is named `form_widget_single_control` in order, as you said, to abstract away if it's an input, select etc. But in fact it can only render `input` and nothing else. So this is misleading. So you could also simply name it `form_widget_input`. Apart from that I agree with everything.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
see #3996 (comment)
ref #3996
The text was updated successfully, but these errors were encountered: