306695 - Boolean charts forgets "0" values

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect, P2)

Description

[Joel Peshkin]

If the value provided in a boolean chart is 0 and the user selects "edit this
search" after running a query, the 0 has become blank.

Comment 1

[Marc Schumann [:Wurblzap]]

Attachment: Patch

Comment 2

[Frédéric Buclin]

Comment on attachment 205857 [details] [diff] [review]
Patch

looks good, but I haven't tested it yet.

Comment 3

[Frédéric Buclin]

Comment on attachment 205857 [details] [diff] [review]
Patch

>         for (my $col = 0; $cgi->param("field$chart-$row-$col"); $col++) {
>+            my $value = $cgi->param("value$chart-$row-$col");
>+            if (!defined($value)) {
>+                $value = '';
>+            }
>             push(@cols, { field => $cgi->param("field$chart-$row-$col"),
>                           type => $cgi->param("type$chart-$row-$col") || 'noop',
>-                          value => $cgi->param("value$chart-$row-$col") || '' });
>+                          value => $value });
>         }

This looks good, and the few tests I did work fine, but... $value is always defined here, because if it is not, you set it to an empty string. And as CGI::canonicalise_query() now keeps defined values, this means you keep *everything*, even fields which were originally undefined. Is that really what we want? IMO, if a $value is undefined, we shouldn't add it to @cols. I now fear that the URL will be even longer than what it is now. And I remember you that we already have a bug about too long URLs (Internet Explorer is limited to ~2000 characters).

r=LpSolit, but I still want joel's review.

Comment 4

[Joel Peshkin]

Looks correct and the rightr way to fix it.  r=joel by inspection.

Comment 5

[Marc Schumann [:Wurblzap]]

Tip:
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.159; previous revision: 1.158
done
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.21; previous revision: 1.20
done

2.22 branch:
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.155.2.1; previous revision: 1.155
done
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.20.2.1; previous revision: 1.20
done