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
Categories
(Bugzilla :: Creating/Changing Bugs, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: caillon, Assigned: caillon)
Details
Attachments
(1 file, 2 obsolete files)
2.48 KB,
patch
|
gerv
:
review+
CodeMachine
:
review+
|
Details | Diff | Splinter Review |
Summary says it all. I have a patch.
Assignee | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
Fwiw, this is applied on http://landfill.tequilarista.org/bz34488/ Review please :)
Assignee | ||
Updated•23 years ago
|
Severity: normal → minor
Target Milestone: --- → Bugzilla 2.16
Updated•23 years ago
|
Attachment #53456 -
Flags: review-
Comment 3•23 years ago
|
||
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•22 years ago
|
||
caillon: this patch is needs-work - are you going to fix it? :-) Gerv
Assignee | ||
Comment 5•22 years ago
|
||
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
Assignee | ||
Comment 6•22 years ago
|
||
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•22 years ago
|
||
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
Attachment #57698 -
Flags: review-
Assignee | ||
Comment 8•22 years ago
|
||
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 9•22 years ago
|
||
Comment on attachment 57720 [details] [diff] [review] Patch v3 r=gerv. Gerv
Attachment #57720 -
Flags: review+
Comment 10•22 years ago
|
||
Yes you was. You said length "wasn't cool". =)
Assignee | ||
Comment 11•22 years ago
|
||
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.
Updated•22 years ago
|
Attachment #57720 -
Flags: review+
Comment 12•22 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•