Item9647: select+values formfields can lose their value mapping

Priority:	CurrentState:	AppliesTo:	Component:	WaitingFor:
Normal	Closed	Engine	DataForms

When creating a Form object using new Foswiki::Form these are cached and thus shared among consecutive calls. So are the gathered _options within a Select formfield. Now, when there's a select+values modifier of the formfield type, an internal valueMap is build up using the key=value pairs in the list of allowed values for that field. This is computed in getOptions() which takes care of the _options cache. All strings in there are also shared by consequtive calls.

Unfortunately these strings in the _options cache are manipulated in place while rendering the html markup.

The second time a Form object is created the old Form object is reused. But now the _options cache has been modified by accident in renderForEdit So now, the call to getOptions() returns a different list of allowed values with the mapping being applied to it.

So far the analysis and the theory.

Here's how to reproduce:

1. install FlexFormPlugin
2. create a DataForm and add a | ListOfThings | select+values | 1 | one=1, two=2, three=3 | | | row to it
3. create a TestTopic and attach the new DataForm to it
4. make a selection for your ListOfThings formfield
5. add

   %RENDERFOREDIT{field="ListOfThings"}%%RENDERFOREDIT{field="ListOfThings"}%

   to it
6. now the error: only the first select box is rendered correctly, the second is wrong

Patch:

Index: Select.pm
===================================================================
--- Select.pm   (revision 8802)
+++ Select.pm   (working copy)
@@ -44,9 +44,11 @@
 sub getOptions {
     my $this = shift;
 
-    return $this->{_options} if $this->{_options};
+    my $vals = $this->{_options};
+    return $vals if $vals;
 
-    my $vals = $this->SUPER::getOptions(@_);
+    $vals = $this->SUPER::getOptions(@_);
+
     if ( $this->{type} =~ /\+values/ ) {
 
         # create a values map
@@ -97,7 +99,8 @@
     my $choices = '';
 
     my %isSelected = map { $_ => 1 } split( /\s*,\s*/, $value );
-    foreach my $option ( @{ $this->getOptions() } ) {
+    foreach my $item ( @{ $this->getOptions() } ) {
+        my $option = $item;
         my %params = ( class => 'foswikiOption', );
         $params{selected} = 'selected' if $isSelected{$option};
         if ( defined( $this->{valueMap}{$option} ) ) {

-- MichaelDaum - 08 Sep 2010

If you need a FlexFormPlugin to see it, and it has been there since (tm)wiki and all Foswiki releases and noone noticed it must be a narrow scope bug.

So I downgrade it to Normal. But you seem to have the fix. Why didn't you check it in to trunk and Release01x01 branch?

-- KennethLavrsen - 08 Sep 2010

My svn checkout is behind and I won't update not to risk my stuff breaking. I need a working wiki all week. So Monday. The bug can also be triggered by two %META{"form"}% I guess.

Agreed, it is only Urgent for me and my client. I can't deploy their latest wiki-app without the above error being hotfixed.

-- MichaelDaum - 08 Sep 2010

Michael,

Is this fix confirmed good? Is this going to get checked in?

-- GeorgeClark - 13 Mar 2011

Thanks George for bringing this up. I will add it to stable + trunk and check the unit tests. The fix should also take Item10071 into account.

-- MichaelDaum - 14 Mar 2011

We are a couple of weeks from 1.1.5 release. Any plans to fix, or defer to 1.2.

-- GeorgeClark - 07 Mar 2012

Working on it...already was fixed in Foswikirev:13616 as part of Item11428. Sorry, lost track of all the +values task items.

-- MichaelDaum - 08 Mar 2012