[webkit-reviews] review denied: [Bug 76893] Implement value sanitization algorithm for type=datetime : [Attachment 169537] Patch

Thu Oct 18 20:27:52 PDT 2012

Kent Tamura <tkent at chromium.org> has denied Kunihiko Sakamoto
<ksakamoto at chromium.org>'s request for review:
Bug 76893: Implement value sanitization algorithm for type=datetime
https://bugs.webkit.org/show_bug.cgi?id=76893

Attachment 169537: Patch
https://bugs.webkit.org/attachment.cgi?id=169537&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=169537&action=review

> LayoutTests/fast/forms/datetime/datetime-value-sanitization-expected.txt:9
> +PASS input.value = "2009-09-07T16:49:00Z"; input.value is
"2009-09-07T16:49Z"
> +PASS input.value = "2009-09-07T16:49:00.000Z"; input.value is
"2009-09-07T16:49Z"

Can you preserve trailing zeros of the input datetime values?

If we do so, we should do:
 - Add "SecondFormat m_preferredSecondForamt;" and "SecondFormat
preferredSecondFormat() const" to DateComponents
 - m_preferredSecondFormat is updated in DateComponents::parseTime
 - DateTimeInputType::sanitizeValue calls
date.toString(date.preferredSecondFormat)

> LayoutTests/fast/forms/datetime/datetime-value-sanitization.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

<!DOCTYPE html>

is enough.

> LayoutTests/fast/forms/datetime/datetime-value-sanitization.html:8
> +<p id="description"></p>
> +<div id="console"></div>

Please remove them.  They are unnecessary nowadays.

> LayoutTests/fast/forms/datetime/datetime-value-sanitization.html:15
> +// Valid values

Such labels are useful for test results too.
So please print it by debug('Valid values');

> LayoutTests/fast/forms/datetime/datetime-value-sanitization.html:16
> +shouldBe('input.value = "2009-09-07T16:49Z"; input.value',
'"2009-09-07T16:49Z"');

should be:
  shouldBeEqualToString('input.value ...', '2009-09-07T16:49Z');
to avoid ugly nested quotes.