[Koha-patches] [PATCH] Bug 3264 UnCloneField() / minus button in MARC editor can clear all subfields (authorities AND biblio)

[Marcel de Rooy]

All subfields following the removed subfield were not saved.
Problem is in C4/Biblio routine TransformHtmlToMarc.
If the field is emptied, the param list contains a code param but no subfield param. The while loop handling the subfields could not handle that.
Also added a FIXME because the whole routine depends on an assumption about the order of cgi parameters that is not strictly guaranteed. 
---
 C4/Biblio.pm |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 9e84ffd..a790f1d 100644
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -2137,6 +2137,7 @@ sub TransformHtmlToMarc {
     my $record = MARC::Record->new();
     my $i      = 0;
     my @fields;
+#FIXME This code assumes that the CGI params will be in the same order as the fields in the template; this is no absolute guarantee!
     while ( $params[$i] ) {    # browse all CGI params
         my $param    = $params[$i];
         my $newfield = 0;
@@ -2176,19 +2177,23 @@ sub TransformHtmlToMarc {
 
                 # > 009, deal with subfields
             } else {
-                while ( defined $params[$j] && $params[$j] =~ /_code_/ ) {    # browse all it's subfield
-                    my $inner_param = $params[$j];
-                    if ($newfield) {
-                        if ( $cgi->param( $params[ $j + 1 ] ) ne '' ) {         # only if there is a value (code => value)
-                            $newfield->add_subfields( $cgi->param($inner_param) => $cgi->param( $params[ $j + 1 ] ) );
-                        }
-                    } else {
-                        if ( $cgi->param( $params[ $j + 1 ] ) ne '' ) {         # creating only if there is a value (code => value)
-                            $newfield = MARC::Field->new( $tag, $ind1, $ind2, $cgi->param($inner_param) => $cgi->param( $params[ $j + 1 ] ), );
-                        }
+                # browse subfields for this tag (reason for _code_ match)
+                while(defined $params[$j] && $params[$j] =~ /_code_/) {
+                    last unless defined $params[$j+1];
+                    #if next param ne subfield, then it was probably empty
+                    #try next param by incrementing j
+                    if($params[$j+1]!~/_subfield_/) {$j++; next; }
+                    my $fval= $cgi->param($params[$j+1]);
+                    #check if subfield value not empty and field exists
+                    if($fval ne '' && $newfield) {
+                        $newfield->add_subfields( $cgi->param($params[$j]) => $fval);
+                    }
+                    elsif($fval ne '') {
+                        $newfield = MARC::Field->new( $tag, $ind1, 
+ $ind2, $cgi->param($params[$j]) => $fval );
                     }
                     $j += 2;
-                }
+                } #end-of-while
+                $i= $j-1; #update i for outer loop accordingly
             }
             push @fields, $newfield if ($newfield);
         }
--
1.6.0.6