You are here: Foswiki>Tasks Web>Item9647 (11 Apr 2012, GeorgeClark)Edit Attach

Item9647: select+values formfields can lose their value mapping

pencil
Priority: Normal
Current State: Closed
Released In: 1.1.5
Target Release: patch
Applies To: Engine
Component: DataForms
Branches: Release01x01
Reported By: MichaelDaum
Waiting For:
Last Change By: GeorgeClark
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 distro:b81fa90080f8 as part of Item11428. Sorry, lost track of all the +values task items.

-- MichaelDaum - 08 Mar 2012
 
Topic revision: r15 - 11 Apr 2012, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy