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

Item9647: select+values formfields can lose their value mapping

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
    to it
  6. now the error: only the first select box is rendered correctly, the second is wrong

---   (revision 8802)
+++   (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


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 - This page was cached on 16 Sep 2021 - 18:57.

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