developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 12 years ago

Closed 12 years ago

#641 closed bug (fixed)

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

Reported by: bibo Owned by:
Priority: normal Milestone: Jelix 1.0.5
Component: jelix:forms Version: 1.0.4
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

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 (5)

comment:1 Changed 12 years ago by laurentj

  • Milestone set to Jelix 1.0.5
  • Version changed from trunk to 1.0.4

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='';
 }

 /**

comment:2 Changed 12 years ago by bibo

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

comment:3 Changed 12 years ago by laurentj

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

comment:4 Changed 12 years ago by bibo

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

comment:5 Changed 12 years ago by laurentj

  • Resolution set to fixed
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.