Problem/Motivation

If a user creates a form element and updates the key to a non-unique value, the form element key edit box disappears. Further to this, when the element edit is closed, the entire form element disappears and on form save the element is deleted.

Steps to reproduce

You can reproduce this behaviour by doing the following:

  1. Enable and configure the webform, form_builder and form_builder_webform modules
  2. Create a new webform
  3. Add a new form element, give it the title Element one and edit the key so it reads non_unique_key
  4. Drag across a second form element, give it the title Element two and edit the key so it reads non_unique_key
  5. Save the form

Expected behaviour

An error message should appear that tells me that I cannot save the form until I edit the keys to make them unique. The error message should be similar or the same as the message provided in Webform module for this validation condition.

Actual behaviour

After clicking anywhere outside the edit box of the second form element, the element disappears and on save, is deleted. No message is shown indicating why this happens.

Proposed resolution

The same scenario, when done in webform without the form_builder_webform module, returns an error on save that reads:
"The field key %key is already in use by the field labeled %label. Please use a unique key."

The Form Builder Webform UI module should display a similar error message, forcing the user to rename the key before continuing.

Remaining tasks

A simpletest test case should be written to test and reproduce this behaviour.

CommentFileSizeAuthor
#8 form_builder-fixup-duplicate-key-validation-1853628-8.patch1.72 KBtorotil
#5 form_builder-1853628-5.patch1.55 KBmarkus_petrux

Comments

thechanceg’s picture

thechanceg commented

Has anything been done on this? Your proposed solution sounds perfect.

I notice that this issue is related to the stable release, has this functionality made it dev yet?

amir simantov’s picture

amir simantov commented

This issue might be closed.

In a fresh install of Surveyor I cannot see this problem; in a case of a duplication I do get an error message "The form key new_textfield is already in use".

Got here by searching "duplicate".

markus_petrux’s picture

markus_petrux commented
Version: 7.x-1.3 » 7.x-1.x-dev

Still an issue here.

Would it work a validation callback attached to the form "form_builder_field_configure"?

...looking at the code, it seems that we want to make sure $form['#_edit_element']['#key'] is unique, but where to look for the keys of the other components that are being edited? Any hint on this?

torotil’s picture

torotil commented

I guess a validation handler on form_builder_field_configure might just work. You can get the currently active form (including all other fields) from form_builder_active_form().

markus_petrux’s picture

markus_petrux commented
Status: Active » Needs review
StatusFileSize
new form_builder-1853628-5.patch1.55 KB

It has not been so obvious to find. I did the best I could...

torotil’s picture

torotil commented
Status: Needs review » Needs work

That might be a bit too strict. A duplicate key is fine as long as it doesn't have the same parent too. For example:

textfield1
fieldset1
  -> textfield1

is a valid form structure.

torotil’s picture

torotil commented

Erm and another thing: I can't reproduce the bug in 7.x-1.x -- also there already is form_builder_property_key_form_validate() which should take care of this - and does so for me.

torotil’s picture

torotil commented
Status: Needs work » Needs review
StatusFileSize
new form_builder-fixup-duplicate-key-validation-1853628-8.patch1.72 KB

Ok still found a bug with that validation: The current validation only checks for duplicate keys in the "global" form array. That means:

  • If a key is used inside a fieldset and in the form directly this gives a false positive.
  • If two fields in a fieldset use the same key it fails to detect it (and the interface breaks).

I'm attaching a patch that fixes the validation.

@markus_petrux: could you check whether that fixes your problems too?

  • torotil committed ac9c9d4 on 7.x-1.x 
    Issue #1853628 by torotil, markus_petrux: Use of a duplicate, non-unique...
torotil’s picture

torotil commented
Status: Needs review » Fixed

Ok, I've committed #8.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.