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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 23 17:38:38 PDT 2009


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

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

------- Additional Comments from TAMURA, Kent <tkent at chromium.org>
> Reversed, no?
>  53	  double maximum;  // maximum must be <= minimum.

Right!	Fixed.

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

Fixed 8 instances in 4 files.

> 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;

I didn't make a such function, and simplified this code by removing
hasAttribute() and .string() instead.  formStringToDouble() returns false if
the input string is null.

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

I improved the code and comments here.


More information about the webkit-reviews mailing list