[webkit-reviews] review granted: [Bug 234039] Allow using a space as date / time separator in <type="datetime-local"> : [Attachment 446628] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 234039: Allow using a space as date / time separator in
<type="datetime-local">
https://bugs.webkit.org/show_bug.cgi?id=234039

Attachment 446628: Patch

https://bugs.webkit.org/attachment.cgi?id=446628&action=review




--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 446628
  --> https://bugs.webkit.org/attachment.cgi?id=446628
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446628&action=review

> Source/WebCore/ChangeLog:9
> +	   This would align our behavior with Gecko and allow us to pass more
tests in WPT.

What about specification?

> Source/WebCore/ChangeLog:21
> +	   - The output will use the shortest possible string, omitting seconds
or milliseconds when 0, as per
> +	    
https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#valid-normalis
ed-local-date-and-time-string

Good, but not mentioned in the bug title.

> Source/WebCore/ChangeLog:27
> +	   Fix bug where we would allow more than 3 digits for the millisecond
part of the time (we
> +	   would silently ignore follow-up digits instead of failing parsing).
This is as per:

Good, but not mentioned in the bug title.

> Source/WebCore/ChangeLog:38
> +	   The output will use the shortest possible string, omitting seconds
or milliseconds when 0, as per
> +	  
https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#valid-normalis
ed-local-date-and-time-string

Good, but not mentioned in the bug title.

> Source/WebCore/platform/DateComponents.cpp:483
>			   // Regardless of the number of digits, we only ever
parse at most 3. All other
>			   // digits after that are ignored, but the buffer is
incremented as if they were
>			   // all parsed.

This comment does not make sense any more and should be deleted.