remove notice when form_id is not set [#169247]

I'm currently working on the Drupal 6 version of the gallery module. One problem I discovered is this:
If you submit a form, that is not generated by Drupal (in this case a Gallery2 form) the index 'form_id' is not available in the #post array. This leads to a warning. The attached patch simply adds a isset($form['#post']['form_id']) check to prevent forms API to handle the 'external' submit.

Comment	File	Size	Author
#2	isset_form_id_2.patch	980 bytes	profix898
#2
	isset_form_id.patch	824 bytes	profix898

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Dries CreditAttribution: Dries commented 21 August 2007 at 16:35

Please add some code comments explaining this (i.e. the part about external forms).

Comment #2

profix898 CreditAttribution: profix898 commented 21 August 2007 at 17:06

File	Size
isset_form_id_2.patch	980 bytes

Comments added. New patch attached.

Comment #3

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 21 August 2007 at 20:59

Well, anyone can set form_id in POST to something, so the comment is not really accurate. The check looks for the form_id and its equivalence with the form_id in the system to check whether it was a Drupal form sumission.

Comment #4

profix898 CreditAttribution: profix898 commented 21 August 2007 at 21:13

"Well, anyone can set form_id in POST to something, so the comment is not really accurate."
But form_id in $_POST must be the same as the current one generated in form_builder(). Why is this inaccurate?

"The check looks for the form_id and its equivalence with the form_id in the system to check whether it was a Drupal form sumission."
Not sure what you mean here!? Do you mean the isset() stuff is redundant? Or are you trying to reword the code comment?

Comment #5

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 21 August 2007 at 21:51

Here is your comment:

Only process the form if $_POST['form_id'] is set (which is not the case for an external - not Drupal generated - form)

As I have said, whether the POST data has a form_id or not is not something which tells "internal" Drupal forms and externals apart. This long winded explanation easily misleads people.

Comment #6

profix898 CreditAttribution: profix898 commented 23 August 2007 at 06:59

whether the POST data has a form_id or not is not something which tells "internal" Drupal forms and externals apart.

I agree that form_id being set does not necessarily mean its a Drupal generated form, but its likely missing if it is not. If you dont want the long comment, what else should I document here? I can only tell you that without the patch every (embedded) non-Drupal form causes a warning on submit. If form_id is set the $form['#post']['form_id'] == $form_id stuff checks whether it needs processing.

Comment #7

Dries CreditAttribution: Dries commented 23 August 2007 at 16:07

Personally, I'm OK with the patch and its comment.

Goba: maybe you can massage the comment, and submit the patch -- unless you think this isn't the proper fix?

Comment #8

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 23 August 2007 at 18:05

Status:

Needs review

» Fixed

OK, let the code intact and modified the comment to say:

  // Only process the form if it is programmed or the form_id coming
  // from the POST data is set and matches the current form_id.

instead of:

  // Only process the form if $_POST['form_id'] is set (which is not the
  // case for an external - not Drupal generated - form) and matches the
  // current form_id.

Note that additionally to what I already noticed, the comment also lacked the programmed form case.

Anyway, thanks for the fix, and excuse me if I was "too picky" about the comment. I think it is important to be precise if possible.