[webkit-reviews] review granted: [Bug 59951] Implement Date and Time Input Value Sanitization : [Attachment 91951] [PATCH] Add Date+Time Value Sanitization

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 59951: Implement Date and Time Input Value Sanitization
https://bugs.webkit.org/show_bug.cgi?id=59951

Attachment 91951: [PATCH] Add Date+Time Value Sanitization
https://bugs.webkit.org/attachment.cgi?id=91951&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=91951&action=review

> Source/WebCore/html/BaseDateAndTimeInputType.cpp:221
> +    double parsedValue = parseLocalizedDate(proposedValue, dateType());
> +    if (isfinite(parsedValue))

I don’t think you need the local variable, nor do I think the local variable’s
name adds much clarity. We can put this all on one line.

> Source/WebCore/html/BaseDateAndTimeInputType.cpp:227
> +    if (!parseToDateComponents(proposedValue, 0))
> +	   return String();
> +
> +    return proposedValue;

I know that often we do early return, but this function seems to return early
in each case where it decides the value is good, except for this last case,
where it does the opposite. I suggest reversing the sense to make the function
overall read better.

Another way to do it would be to factor this into a function that returns a
boolean and then this function would just call that and then return the
appropriate string. It’s a little strange that the code in this function is not
sanitizing, just rejecting values that can’t be parsed.