[webkit-reviews] review denied: [Bug 27968] [HTML5][Forms] <input type=number> UI : [Attachment 39540] Patch part 4: Rendering of spin buttons and behavior for type=number

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 27968: [HTML5][Forms] <input type=number> UI
https://bugs.webkit.org/show_bug.cgi?id=27968

Attachment 39540: Patch part 4: Rendering of spin buttons and behavior for
type=number
https://bugs.webkit.org/attachment.cgi?id=39540&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Quick pass with some things to fix. 

Please take each comment as something to look for throughout the patch (as
things usually occur more than once).

Your changelog seems a bit sparse.
Please remove commented out code.
Please remove printf's.

>+ Test for the spin control behavior in a type=numnber input.
typo: numnber

> + #define EPSILON (16.0 * std::numeric_limits<double>::epsilon())

Why is this a define instead of a const double? Also epsilon doesn't seem like
the right name since it is 16 * epsilon.

> + void HTMLInputElement::stepUpFromUI(int n)
"n" A more meaningful variable name would be better.


> + void paintBoxDecorationsWithSize(PaintInfo&, int, int, int, int);

Don't include parameter names if they don't add information but in this case
the names for the int parameters would add a lot of information.

This occurs in several places but I just picked out one.

> + // Center the spin button vertically, and move it to the right
Finish sentences with "."

> + const MouseEvent* mevt = static_cast<MouseEvent*>(evt);
"mevt" Use whole words.