[webkit-reviews] review denied: [Bug 29069] [HTML5][Forms] min/max attributes support for type=number : [Attachment 39858] Proposed patch (rev.2)

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 29069: [HTML5][Forms] min/max attributes support for type=number
https://bugs.webkit.org/show_bug.cgi?id=29069

Attachment 39858: Proposed patch (rev.2)
https://bugs.webkit.org/attachment.cgi?id=39858&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Reversed, no?
 53	double maximum;  // maximum must be <= minimum.

Tab, will cause commit to fail:
 13	testPassed(resultText);

I would have probably written a little static inline function to help grab the
numbers, but I think my "make a function" threshold is lower than most others:
	 double min;
 202	     double doubleValue;
 203	     if (hasAttribute(minAttr) &&
formStringToDouble(getAttribute(minAttr).string(), &min) &&
formStringToDouble(value(), &doubleValue))
 204		 return doubleValue < min;

This comment mostly confuses me:
 249		     max = min;  // The spec allows min == max.
I'm not surprised that min is allowed to equal max.

Why not write that whole if as:

max = max(min, max(100, max))
obviously you'd have to use std:: or change the name of max.

r- for the nits.  Really only the backwards comment, the rest doesn't need to
be changed.  If you were a committer, I would r+ this and you could commit it
with mods yourself.