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 10 years ago

Closed 10 years ago

#1234 closed bug (fixed)

jForms: modified values are not well detected for listbox and checkboxes

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

Description

The problem comes from $form->getModifiedControls() here: http://jelix.org/reference/trunk/__filesource/fsource_jelix_forms_formsjFormsBase.class.php.html#a600 In the comment it is said

"so we have to compare by ourself."

In this case, it's only a docneedeed problem, because in the doc, we don't tell that this functionnality doesn't work for listbox and checkboxes. But I think it would be possible to make it work.

But despite this comment, the code still use array_diff in case of 2 arrays (case exclusive for listbox and checkboxes, they are the only controls to use arrays). Currently the problem is that the array_diff function don't work with numerical indexes: example:

$v1 = array(0=>'1');
$v2 = array(0=>'1', 1=>'2');
var_dump(array_diff($v1, $v2));

result in an empty array

$v1 = array("a" => "green", "red", "blue", "red");
$v2 = array("b" => "green", "yellow", "red");
var_dump(array_diff($v1, $v2));

result in an array (1 => 'blue')

But as we only want to know if the values has been modified, we should replace array_diff by a simple comparison

$v1!==$v2

There is 2 other bugs:

  • For listbox and checkboxes, if no values preselected, after a jForms::create(), container->data contains an empty array, and after a jForms::fill, the empty arrays becomes empty strings (if no values where selected), then getModifiedControls detect it has been modified, althrought it has not.
  • The jFormsControlsubmit control is always empty after a jForms::create(), and is detected modified since after the jForms::fill it contains a value.

Last but not least, we should make some tests for checkboxes and listbox.

I'm not sure to have well explained the problem. Currently investigating on it but not sure at all to have the level of debugging this, but I'll try my best.

Change History (7)

comment:1 Changed 10 years ago by catsoup

I made a mistake for array_diff, it works well with numerical values. But for our current use, it's not well adapted because it's normal behaviour is to return the entry in array1 that are not present in others arrays, so:

$v1 = array();
$v2 = array (0=>'bbb', 1 =>'123');
var_dump(array_diff($v1, $v2));

returns an empty array

$v1 = array (0=>'bbb', 1=>'123');
$v2 = array();
var_dump(array_diff($v1, $v2));

returns array (0=>'bbb', 1 =>'123')

comment:2 Changed 10 years ago by catsoup

Made tests that fails: http://bitbucket.org/catsoup/jelix-trunk-patches/src/tip/ticket1234-tests.patch And the patch: http://bitbucket.org/catsoup/jelix-trunk-patches/src/tip/ticket1234.patch

There is another point I would like to improve: the fact that the jFormsControlSubmit control is not in the data container, but it might be for good reasons, so not sure it's a good idea :)

comment:3 Changed 10 years ago by catsoup

  • review set to review?

Ok for the jFormsControlSubmit it is not a bug, we can have more that one submit, so there's no defaulvalue.

comment:4 Changed 10 years ago by laurentj

the patch could be ok. But with this another new test, it fails :

        $this->form->setData('list', array('123','aaa'));
        $this->form->initModifiedControlsList();
        $this->form->setData('list', array('aaa', '123'));
        $modifiedControls = array ();
        $this->assertIdentical($modifiedControls, $this->form->getModifiedControls());

However I don't know if we should consider the order as an important thing. It could be in some situation, and not important in others. Anyway, the test is about the old behavior (the order is not discriminant), and I think we should keep the old behavior, at least on 1.1.x. But array_diff is "buggied"...

comment:5 Changed 10 years ago by catsoup

Yes, personally, I would not consider the order important.
Sorry I failed to notice to test this case.
But I found a nice way to do a double-sided array_diff, it comes from "viking coder" at this url: http://www.navioo.com/php/docs/function.array-diff.php it works great if values have been added or removed, and it doesn't report a change if only the order has been modified.

I added your test here: http://bitbucket.org/catsoup/jelix-trunk-patches/src/647ed1b67b03/ticket1234-tests.patch
And you can see the patch here: http://bitbucket.org/catsoup/jelix-trunk-patches/src/647ed1b67b03/ticket1234.patch

comment:6 Changed 10 years ago by laurentj

  • review changed from review? to review+

nice :-)

comment:7 Changed 10 years ago by laurentj

  • Milestone changed from Jelix 1.2 to Jelix 1.1.8
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.