WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 91481
Decimal::toString should not round integer value.
https://bugs.webkit.org/show_bug.cgi?id=91481
Summary
Decimal::toString should not round integer value.
yosin
Reported
2012-07-17 03:00:32 PDT
Following test case in LayoutTests/fast/forms/datetime/input-valueasnumber-datetime.html is failed: shouldBe('setValueAsNumberAndGetValue(275760, 8, 12, 0, 0, 0, 1)', '"275760-09-12T00:00:00.001Z"'); The root cause is Decimal::toString. It converts an integer 8639999913600001 (= Date.UTC(275760, 8, 12, 0, 0, 0 1)) to a string "8639999913600000", the last digit "1" is converted to digit "0".
Attachments
Patch 1
(3.67 KB, patch)
2012-07-17 03:09 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Patch 2
(4.46 KB, patch)
2012-07-17 19:22 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Patch 3
(4.77 KB, patch)
2012-07-18 00:23 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
yosin
Comment 1
2012-07-17 03:09:12 PDT
Created
attachment 152724
[details]
Patch 1
yosin
Comment 2
2012-07-17 03:11:40 PDT
Comment on
attachment 152724
[details]
Patch 1 Could you review this patch? Thanks in advance. P.S. Changes for Decimal.cpp is only adding "if (originalExponent < 0)" and indentation. git diff -w Decimal.cpp shows two line changes.
Kent Tamura
Comment 3
2012-07-17 03:40:00 PDT
Comment on
attachment 152724
[details]
Patch 1 View in context:
https://bugs.webkit.org/attachment.cgi?id=152724&action=review
> Source/WebCore/ChangeLog:14 > + (WebCore::Decimal::toString): When the value is an integer, we don't > + round coefficient to DBL_DIG(15) digits.
Why do we need the rounding for non-integer cases?
> Source/WebKit/chromium/tests/DecimalTest.cpp:1045 > + // 8639999913600001=Date.UTC(275760, 8, 12, 0, 0, 0 1)
Understing it helps nothing. Please remove the comment.
> Source/WebKit/chromium/tests/DecimalTest.cpp:1046 > + EXPECT_STREQ(("8639999913600001"), encode(8639999913600001, 0, Positive).toString().ascii().data());
We need more test cases. For example, MaxCoefficient with various exponents.
yosin
Comment 4
2012-07-17 19:22:20 PDT
Created
attachment 152904
[details]
Patch 2
yosin
Comment 5
2012-07-18 00:23:12 PDT
Created
attachment 152949
[details]
Patch 3
yosin
Comment 6
2012-07-18 00:29:52 PDT
(In reply to
comment #3
)
> (From update of
attachment 152724
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=152724&action=review
> > > Source/WebCore/ChangeLog:14 > > + (WebCore::Decimal::toString): When the value is an integer, we don't > > + round coefficient to DBL_DIG(15) digits. > > Why do we need the rounding for non-integer cases?
The reason is we would like to get "1" for 1/3 * 3 or 1/256 * 256. Using DBL_DIG is chosen by converting Decimal to double, e.g. HTMLInputElement.valueAsNumber method.
yosin
Comment 7
2012-07-18 00:31:11 PDT
Comment on
attachment 152949
[details]
Patch 3 Could you review this patch? Thanks in advance. = Changes since the last review = * Added more tests as suggested. * Use newly introduced macro EXPECT_DECIMAL_STREQ.
Kent Tamura
Comment 8
2012-07-18 00:45:25 PDT
Comment on
attachment 152949
[details]
Patch 3 ok
yosin
Comment 9
2012-07-18 01:09:53 PDT
Comment on
attachment 152949
[details]
Patch 3 Clearing flags on attachment: 152949 Committed
r122928
: <
http://trac.webkit.org/changeset/122928
>
yosin
Comment 10
2012-07-18 01:09:59 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug