Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addGroup inconsistent behaviour #16

Open
MaybeFactor opened this issue Aug 2, 2018 · 3 comments
Open

addGroup inconsistent behaviour #16

MaybeFactor opened this issue Aug 2, 2018 · 3 comments

Comments

@MaybeFactor
Copy link

Hi again,
When adding a group of elements, the behaviour of the addGroup method has some inconsistency in the resulting html element depending on whether or not a $name is specified. Basically, if a name is provided, the $appendName parameter is respected (defaulting to true), but if no name is specified, a name gets generated and $appendName is set to false. Having recently updated our code to flack/quickform from the old pear version, this is a difference in behaviour that makes updating more risky.

e.g.

$group[] = $form->createElement("text", "field");
$form->addGroup($group);

produces <input type="text" name="field" />, but

$group[] = $form->createElement("text", "field");
$form->addGroup($group, "group");

produces <input type="text" name="group[field]" />.

An easy fix is to change the default $appendName to false, so that the default behaviour is universally to not append the names. I'm happy to apply this change myself, but wanted to make sure other contributors were agreed that it's a good idea. Let me know if you think this is alright, and I'll go ahead with the change.

@flack
Copy link
Collaborator

flack commented Aug 2, 2018

hm, strange. This shouldn't be different between this version and the pear version:

https://github.com/pear/HTML_QuickForm/blob/trunk/QuickForm.php#L757-L760

but you're saying that the PEAR version produces different markup in this case? Can you post an example?

@MaybeFactor
Copy link
Author

Hmm, you're right... the original pear/quickform code for addGroup looks identical. I don't want to post any proprietary code for examples, so I built the examples in the original post to demonstrate this.

Prior to upgrading to flack/quickform, we were able to use the POSTed variable array to directly populate a domain object or insert to the database. Maybe the difference isn't caused when adding groups, but when building up that array. e.g. the second example above actually produces a nested array, like so

array (
    'group' => array (
        'field' => 'field_value'
    )
)

I don't think the old pear version did that.

@flack
Copy link
Collaborator

flack commented Aug 3, 2018

Could you test with version 3.3.3? Recently, we ported some security changes related to how nested fields are handled. Maybe what you're seeing is a side effect of that

P.S.: but just looking at the code you posted

<input type="text" name="group[field]" />

should definitely produce POST data like this:

array (
    'group' => array (
        'field' => 'field_value'
    )
)

i.e. that is the default PHP behavior. TBH, I'm not exactly sure if HTML_Quickform is supposed to transfer this to some other structure (it sounds error-prone, because if you have group1['field'] and group2['field'], you cannot simply flatten the array to just have field. It wouldn't know which data to select). But like I said, a test with 3.3.3 might be interesting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants