Bug 91469 - [Forms] Test expectations of datetime/datetime-local/time should not contain milliseconds when they aren't expected
Summary: [Forms] Test expectations of datetime/datetime-local/time should not contain ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-16 22:10 PDT by yosin
Modified: 2012-07-17 00:23 PDT (History)
1 user (show)

See Also:


Attachments
Patch 1 (8.66 KB, patch)
2012-07-16 22:42 PDT, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-07-16 22:10:18 PDT
Following tests have wrong test expectations:
 * fsat/from/datetime/datetime-stepup-stepdown-from-renderer.html
 * fsat/from/datetimelocal/datetimelocal-stepup-stepdown-from-renderer.html
 * fsat/from/time/time-stepup-stepdown-from-renderer.html

For very big step value like:
  shouldBe('stepUp("2012-02-12T20:13Z", "3.40282346e+38", null)', '"276760-09-13T00:00:00.000Z"');

In this case, an expectation should not have milliseconds, e.g. "276760-09-13T00:00:00".

Before r119948 (introducing Decimal arithmetic), BasseDateAndTimeInputType::serialize checked
whether resulted string containing milliseconds or not by using fmod(step, msecPerSecond), where
msecPerSecond is 1000.

This expression works fine when step isn't big number, and fmod(3.40282346e+38, 1000) returns 616
on my x86 linux box.

Since r119948, BaseDateAndTimeInput::serialize uses same expression but it is done by decimal arithmetic,
e.g. step.remainder(msecPerSecond).isZero(), and this expression returns false for big number.
Comment 1 yosin 2012-07-16 22:42:35 PDT
Created attachment 152697 [details]
Patch 1
Comment 2 yosin 2012-07-16 22:43:27 PDT
Comment on attachment 152697 [details]
Patch 1

Could you review this patch?
Thanks in advance.
Comment 3 Kent Tamura 2012-07-16 23:54:57 PDT
Comment on attachment 152697 [details]
Patch 1

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

> LayoutTests/fast/forms/datetime/datetime-stepup-stepdown-from-renderer-expected.txt:18
> +PASS stepUp("2010-02-10T20:13Z", "3.40282346e+38", null) is "275760-09-13T00:00:00Z"

Why don't the result 275760-09-13T00:00Z ?
Comment 4 yosin 2012-07-17 00:15:45 PDT
(In reply to comment #3)
> (From update of attachment 152697 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152697&action=review
> 
> > LayoutTests/fast/forms/datetime/datetime-stepup-stepdown-from-renderer-expected.txt:18
> > +PASS stepUp("2010-02-10T20:13Z", "3.40282346e+38", null) is "275760-09-13T00:00:00Z"
> 
> Why don't the result 275760-09-13T00:00Z ?

In BaseDateAndTimeInputType::serializeWithComponents which called by BaseDateAndTimeInputType::serialize checks step value against millisecond per minute and second per millisecond as below:
    ...
    if (step.remainder(msecPerMinute).isZero())
        return date.toString(DateComponents::None);
    if (step.remainder(msecPerSecond).isZero())
        return date.toString(DateComponents::Second);
    return date.toString(DateComponents::Millisecond);

In this test case, step value mod(3.40282346e+38, msecPerSecond) != 0 and mod(3.40282346e+38, 1000) == 0. So, we call DateComponent::toString with DateComents::Second argument.

This is reason that we have "00:00:00" (including second) instead of "00:00" (no second), even if second is zero.
Comment 5 Kent Tamura 2012-07-17 00:20:05 PDT
Comment on attachment 152697 [details]
Patch 1

ok
Comment 6 yosin 2012-07-17 00:23:26 PDT
Comment on attachment 152697 [details]
Patch 1

Clearing flags on attachment: 152697

Committed r122815: <http://trac.webkit.org/changeset/122815>
Comment 7 yosin 2012-07-17 00:23:32 PDT
All reviewed patches have been landed.  Closing bug.