Closed Bug 104677 Opened 23 years ago Closed 22 years ago

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


(Bugzilla :: Creating/Changing Bugs, defect)

Not set



Bugzilla 2.16


(Reporter: caillon, Assigned: caillon)



(1 file, 2 obsolete files)

Summary says it all.  I have a patch.
Fwiw, this is applied on

Review please :)
Keywords: patch, review
Severity: normal → minor
Target Milestone: --- → Bugzilla 2.16
Attachment #53456 - Flags: review-
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

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
Keywords: patch, review
caillon: this patch is needs-work - are you going to fix it? :-)

Attached patch Proposed patch v2.0 (obsolete) — Splinter Review
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.
Attachment #53456 - Attachment is obsolete: true
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
Keywords: patch, review
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

>+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?

Attachment #57698 - Flags: review-
Attached patch Patch v3Splinter Review
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.
Attachment #57698 - Attachment is obsolete: true
Comment on attachment 57720 [details] [diff] [review]
Patch v3


Attachment #57720 - Flags: review+
Yes you was.  You said length "wasn't cool". =)
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.
Attachment #57720 - Flags: review+
Checking in;
/cvsroot/mozilla/webtools/bugzilla/,v  <--
new revision: 1.129; previous revision: 1.128
Checking in showvotes.cgi;
/cvsroot/mozilla/webtools/bugzilla/showvotes.cgi,v  <--  showvotes.cgi
new revision: 1.10; previous revision: 1.9
Closed: 22 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.