104677 - vote field for products which allow multiple votes defaults to size="5" and doesnt include a maxlength="" attribute

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Summary says it all.  I have a patch.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch to figure out the max number of digits that votes can have

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Fwiw, this is applied on http://landfill.tequilarista.org/bz34488/

Review please :)

Comment 3

[Matthew Tuck [:CodeMachine]]

I suggest we do the following to the patch:

That min stuff should use a int-min routine.  If we don't have one in Perl, add
one to globals.pl.

The log stuff should either be changed to use standard Perl facilities (I think
conciseness is more important than performance here), or made reusable and added
to globals.pl.

Comment 4

[Gervase Markham [:gerv]]

caillon: this patch is needs-work - are you going to fix it? :-)

Gerv

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Proposed patch v2.0

Yeah I had this done over the weekend and couldn't get to diff it since cvs was
down.  Forgot about it.  Here's the new patch.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Open for review again.

Added the min/max routine like Matty suggested.  Also, log10 is not a built-in
function, so i made my own quasi log10 function.  It's really a log10 + 1
function, but since it returns the digits, I just called it digits.

And for kicks, I applied this patch on http://landfill.tequilarista.org/bz104677/

Comment 7

[Gervase Markham [:gerv]]

Comment on attachment 57698 [details] [diff] [review]
Proposed patch v2.0

Is this patch really worth doing? What _bug_ does it fix? Is this really a
problem?

>+sub digits {
>+    my $number = $_[0];
>+    my $maxlength = 0;
>+    for ($maxlength; $number > 1; $maxlength++) {
>+        $number = int($number / 10);
>+    }
>+    return $maxlength;
>+}

Better done by taking the length of the variable - this converts it to a
string. Do you even need a subroutine?

Gerv

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v3

Gerv, it is not a huge bug but it is one nonetheless.  We shouldn't take more
user input than we can use.  Currently, on products which allow multiple votes,
a user can enter an arbitrarily large number.  For example, if we only allow 5
votes per bug, the user is able to still enter (ridiculous example:)
35108356468435409840684638445140684351498946510684384973 into the field. 
Regardless of what sanity checks on the input we have, it is still sloppy to
allow it to be entered in the first place.

We know the maximum number of votes a product can have, and thus should allow
only as many characters as there are digits.  if it's only allowed 1 vote, it's
a checkbox, and this doesn't come into play.

Should have used length to begin with.	Wasn't thinking.  This updated patch
should be ready if that was the only problem with it.

Comment 9

[Gervase Markham [:gerv]]

Comment on attachment 57720 [details] [diff] [review]
Patch v3

r=gerv.

Gerv

Comment 10

[Matthew Tuck [:CodeMachine]]

Yes you was.  You said length "wasn't cool". =)

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

yeah well, it's not ;-)  why use built in functions when you can make your own?  ;-P

heh i meant to switch it but forgot.

Comment 12

[Matthew Tuck [:CodeMachine]]

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.129; previous revision: 1.128
done
Checking in showvotes.cgi;
/cvsroot/mozilla/webtools/bugzilla/showvotes.cgi,v  <--  showvotes.cgi
new revision: 1.10; previous revision: 1.9
done