Ticket #641 (closed bug: fixed)

Opened 2 months ago

Last modified 1 month ago

jFormsControlDataSource should not define defaultValue=array() as it is not valid on a <menulist>

Reported by: bibo Assigned to:
Priority: normal Milestone: Jelix 1.0.5
Component: jelix:forms Version: 1.0.4
Severity: normal Keywords:
Cc: Php version:
Review: Hosting Provider:
Documentation needed: 0 Blocking:

Description

This is theorically invalid to assign an empty array to a menulistcontrol as it only selects one value.

Practically, a bug can appear as the developper is waiting for a single value and not an array. BUT it occured to me on what i think is rather a rare case. Below is a description:

  • a <menulist ref="select"> control is deactivated in my form because of a preceding choice
  • on submit, my code is factorized so it treats all choices (select deactivated or not) and don't use prepareDaoFromControls. Its loops over form data and assign them to corresponding property of a dao (with additional processing on some).
  • if <select> is deactivated, jelix will throw warning in for example mysql_real_escape_string on a dao insert/update.

Change History

07/16/08 00:11:55 changed by laurentj

  • version changed from trunk to 1.0.4.
  • milestone set to Jelix 1.0.5.

I think a patch like this one is ok:

Index: lib/jelix/forms/jFormsCompiler.class.php
===================================================================
--- lib/jelix/forms/jFormsCompiler.class.php    (révision 1016)
+++ lib/jelix/forms/jFormsCompiler.class.php    (copie de travail)
@@ -243,7 +243,11 @@
                     $source[]='$ctrl->defaultValue='.$str.');';
                     $hasSelectedValues = true;
                 }elseif(isset($control['selectedvalue'])){
-                    $source[]='$ctrl->defaultValue=array(\''. str_replace("'","\\'", (string)$control['selectedvalue']) .'\');';
+                    if ($controltype == 'menulist' ||  $controltype == 'radiobuttons') {
+                        $source[]='$ctrl->defaultValue=\''. str_replace("'","\\'", (string)$control['selectedvalue']) .'\';';
+                    } else {
+                        $source[]='$ctrl->defaultValue=array(\''. str_replace("'","\\'", (string)$control['selectedvalue']) .'\');';
+                    }
                     $hasSelectedValues = true;
                 }
             case 'submit':
Index: lib/jelix/forms/jFormsControl.class.php
===================================================================
--- lib/jelix/forms/jFormsControl.class.php     (révision 1016)
+++ lib/jelix/forms/jFormsControl.class.php     (copie de travail)
@@ -132,6 +132,7 @@
  */
 class jFormsControlRadiobuttons extends jFormsControlDatasource {
     public $type="radiobuttons";
+    public $defaultValue='';
 }

 /**
@@ -180,6 +181,7 @@
  */
 class jFormsControlMenulist extends jFormsControlDatasource {
     public $type="menulist";
+    public $defaultValue='';
 }

 /**

07/16/08 09:49:01 changed by bibo

review+ :-) Tell if you want me to push it.

07/16/08 10:10:35 changed by laurentj

review- : I think tests have to be updated :-)

07/16/08 10:29:22 changed by bibo

Ahh, those tests ! I'll take care of them

07/18/08 23:18:36 changed by laurentj

  • status changed from new to closed.
  • resolution set to fixed.

patch landed into the branch 1.0.x, and the trunk.

Download in other formats: Comma-delimited Text Tab-delimited Text RSS Feed