Bug 80009 – [Forms] Introduce Decimal arithmetic to fix rounding errors in number/range input types

• Component: Forms
• Status: RESOLVED FIXED

[Kent Tamura]

Try:
data:text/html,<input type=range step=0.1 value=0.6 ><script>document.write(document.getElementsByTagName('input')[0].value);</script>

It should show '0.6', not '0.6000000000000001'


This was found by IE Test Center:
http://samples.msdn.microsoft.com/ietestcenter/#html5Range
"Set the 'value' to any numeric value in markup"

[Sameer Patil]

This issue looks to be because of wrong step factor representation in double value. Here step factor is "0.1" when it is converted from string to double it is represented as "0.10000000000000001".

Can anyone please confirm whether it is related to representation error mentioned here http://www.network-theory.co.uk/docs/pytut/RepresentationError.html.

[Kent Tamura]

(In reply to comment #1)
> This issue looks to be because of wrong step factor representation in double value. Here step factor is "0.1" when it is converted from string to double it is represented as "0.10000000000000001".
> 
> Can anyone please confirm whether it is related to representation error mentioned here http://www.network-theory.co.uk/docs/pytut/RepresentationError.html.

Yeah, this is caused by the representation error in IEEE754.
We should update RangeInputType::sanitizeValue() like:

  If proposedValue satisfies min/max/step restrictions,
    return proposedValue
  return serializeForNumberType(StepRange(element()).clampValue(proposedValue));

Add another sample, it displays
0
0.1
0.2
0.30000000000000004
0.4
0.5
0.6000000000000001
0.7000000000000001
0.8
0.9

It is better to change double print function to use "%.15f"?

#include <stdio.h>
int main(int, char**) {
   for (int i = 1; i <= 9; i++) {
       double x = i * 0.1;
       printf("%d %f %.15f %.16f %.17f\n", i, x, x, x, x);
   }
   return 0;
}

1 0.100000 0.100000000000000 0.1000000000000000 0.10000000000000001
2 0.200000 0.200000000000000 0.2000000000000000 0.20000000000000001
3 0.300000 0.300000000000000 0.3000000000000000 0.30000000000000004
4 0.400000 0.400000000000000 0.4000000000000000 0.40000000000000002
5 0.500000 0.500000000000000 0.5000000000000000 0.50000000000000000
6 0.600000 0.600000000000000 0.6000000000000001 0.60000000000000009
7 0.700000 0.700000000000000 0.7000000000000001 0.70000000000000007
8 0.800000 0.800000000000000 0.8000000000000000 0.80000000000000004
9 0.900000 0.900000000000000 0.9000000000000000 0.90000000000000002

[Kent Tamura]

(In reply to comment #3)
> It is better to change double print function to use "%.15f"?

I don't think so. 
A simple fix is to avoid resetting a valid value as I wrote in Comment 2.
Another fix is to use HTMLInputElement::alignValueForStep() or something like it.

[Sameer Patil]

> Another fix is to use HTMLInputElement::alignValueForStep() or something like it.
More details on similar issue logged for number input element https://bugs.webkit.org/show_bug.cgi?id=48308

How about using std::decimal (aka ISO C++ TR24733[1]) available GCC 4.5.
decimal128 supports 
  exponent -6143 to 6144
  digits 34

It is wider than double(float64).

[1] http://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.tr24733

Until all compilers support decimal128, we should have third party one or implemented by ourselves, not full version.

Created attachment 136870 [details]
Patch 1 - for Preliminary Review

[WebKit Review Bot]

Attachment 136870 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/chromium/tests/Decimal128Test.cpp:97:  uint64_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/platform/Decimal128.cpp:127:  Missing spaces around /  [whitespace/operators] [3]
Source/WebCore/platform/Decimal128.cpp:129:  Missing spaces around ==  [whitespace/operators] [3]
Source/WebCore/platform/Decimal128.cpp:130:  Missing spaces around !=  [whitespace/operators] [3]
Source/WebCore/platform/Decimal128.cpp:131:  Missing spaces around <  [whitespace/operators] [3]
Source/WebCore/platform/Decimal128.cpp:132:  Missing spaces around <=  [whitespace/operators] [3]
Source/WebCore/platform/Decimal128.cpp:134:  Missing spaces around >=  [whitespace/operators] [3]
Source/WebCore/platform/Decimal128Impl.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/Decimal128Impl.cpp:92:  uint64_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 9 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Comment on attachment 136870 [details]
Patch 1 - for Preliminary Review

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

Would you show how many bytes will the WebCore (or a browser) size increase by?

> Source/WebCore/platform/Decimal128.cpp:13
> + * 1.    Redistributions of source code must retain the above copyright
> + *         notice, this list of conditions and the following disclaimer.
> + * 2.    Redistributions in binary form must reproduce the above copyright
> + *         notice, this list of conditions and the following disclaimer in the
> + *         documentation and/or other materials provided with the distribution.
> + *

This is not a copyright header which Google employees usually use.

> Source/WebCore/platform/Decimal128.h:50
> +    Decimal128 operator +(double) const;

I don't think we need operators for all of these types (double, int32_t, int64_t, uint32_t, uint64_t).  Please remove unnecessary operators.

> Source/WebCore/platform/Decimal128Impl.cpp:37
> +namespace {

We don't use anonymous namespace in WebKit.

> Source/WebCore/platform/Decimal128Impl.cpp:71
> +static const int kDpdBits = 10;
> +static const int kDpdMask = (1 << kDpdBits) - 1; // = 0x3FF

We don't use k-prefixes.
We use all-capital or all-lowercase letters for acronyms.
http://www.webkit.org/coding/coding-style.html#names-basic

[Kent Tamura]

Applying decimal arithmetic to solve this issue is probably nice, but we don't need compliance with IEEE754 decimal128 at all.  We need a small code to solve this issue.

Created attachment 137264 [details]
Patch 2 - for Preliminary Review

[Kent Tamura]

Comment on attachment 137264 [details]
Patch 2 - for Preliminary Review

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

Please do not set r? for an incomplete patch.
I hesitate to review this patch until the patch actually resolves the type=number/range issues.


> Source/WebCore/platform/Decimal128Impl.cpp:42
> +namespace {

We don't use anonymous namespace.

> Source/WebCore/platform/Decimal128Impl.cpp:164
> +class BcdNum {

This should be BCDNumber or something.
http://www.webkit.org/coding/coding-style.html#names-basic
http://www.webkit.org/coding/coding-style.html#names-full-words

Created attachment 139169 [details]
Patch 3 - for Preliminary Review

[Early Warning System Bot]

Comment on attachment 139169 [details]
Patch 3 - for Preliminary Review

Attachment 139169 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12554172

[Build Bot]

Comment on attachment 139169 [details]
Patch 3 - for Preliminary Review

Attachment 139169 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12549181

[Kent Tamura]

Comment on attachment 139169 [details]
Patch 3 - for Preliminary Review

(In reply to comment #12)
> Please do not set r? for an incomplete patch.

Comment on attachment 139169 [details]
Patch 3 - for Preliminary Review

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

> Source/WebCore/ChangeLog:1
> [phone]  Yoshifumi Inoue  <[e-mail]>

You should add Decimal.cpp and Decimal.h to WebCore/WebCore.vcproj for Windows build.

> Source/WebCore/html/RangeInputType.cpp:248
> +        setValueAsNumber(parseToDouble(newValue.toString(), numeric_limits<double>::quiet_NaN()), eventBehavior, ec);

You should call element()->setValue rather than converting Decimal to double. RangeInputType::setValueAsNumber == element()->setValue(serialize(value), eventBehavior). Note: It doesn't pass ExcpetionCode to element()->setValue. Because of handleKeydownEvent method doesn't care about exception during setValue.

> Source/WebCore/html/StepRange.cpp:44
> +    maximum = Decimal::fromString(serializeForNumberType(element->maximum()));

StepRange class should retrieve maximum and minimum from HTML attribute and RangeType::maximum and minimum use StepRange class.

> Source/WebCore/html/StepRange.cpp:65
> +    return value.isFinite() ? clampValue(value) : clampValue((minimum + maximum) / 2);

We should use StepRange::defaultValue which is clampValue((minimum + maximum) / 2).

> Source/WebCore/html/StepRange.cpp:74
> +        return clampValue((minimum + maximum) / 2);

We should use StepRange::defaultValue which is clampValue((minimum + maximum) / 2).

> Source/WebCore/platform/Decimal.cpp:49
> +static const uint64_t MaxCoefficient = 0x16345785D89FFFF; // 999999999999999999; // 18 9's

We should add "ull" suffix. The compiler used in Qt build doesn't accept this.

> Source/WebCore/platform/Decimal.cpp:753
> +                }

We should count number of extra digits and add to result exponent.

> Source/WebCore/platform/Decimal.cpp:859
> +            if (ch == '-') {

Leading "+" sign isn't not "valid floating point number" in HTML5 spec (2.5.4.3 Floating-pointer numbers).
However, "rules for parsing floating point number values" allows parse string contains "+" sign.
We'll file a bug about this and change Decimal::fromString and parseToDoubleForNumberType.
Note: WTFString::toDouble (StringToDoubleConverter::StringToDouble) accepts "+".

> Source/WebCore/platform/Decimal.cpp:903
> +            return zero(0);

We should return NaN if result is not (2^-1024, 2^1024).
DBL_MIN_10_EXP=-307
least-positive-normalized-double-float=2.2250738585072014d-308
(integer-decode-float least-positive-normalized-double-float) = significand=4503599627370496(==1<<52), exponent=-1074=-1024-53

> Source/WebCore/platform/Decimal.cpp:913
> +        return Decimal(accumulator, resultExponent, minus);

We should return NaN if result is not (2^-1024, 2^1024).
DBL_MAX=1.7976931348623157e+308

> Source/WebCore/platform/Decimal.cpp:956
> +Decimal Decimal::roundAt(int numberOfDigits)

Please remove unused method Decimal::roundAt.

> Source/WebCore/platform/Decimal.cpp:981
> +        return sign() ? "-Infinity" : "Infinity";

Terms "Infinity" and "NaN" come from IEEE 754 specification and they are also found in http://trac.webkit.org/browser/trunk/Source/WTF/wtf/dtoa/double-conversion.cc

> Source/WebCore/platform/Decimal.h:122
> +    Decimal roundAt(int);

Please remove unused method Decimal::roundAt.

For Decimal::fromString, how about calling parseToDoubleForNumberType to check "valid floating-point number"? If so, we can share validity checking.

Comment on attachment 139169 [details]
Patch 3 - for Preliminary Review

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

> Source/WebCore/platform/Decimal.cpp:116
> +        x /= 10;

We should use multiplication instead of division, or table look up.

Created attachment 139584 [details]
Patch 4

[Build Bot]

Comment on attachment 139584 [details]
Patch 4

Attachment 139584 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12584528

Created attachment 139585 [details]
Patch 5

[Kent Tamura]

Let's change the summary to match to what we do.

[Kent Tamura]

Comment on attachment 139585 [details]
Patch 5

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

Don't you apply Decimal to NumberInputType?

> Source/WebCore/ChangeLog:62
> +        * html/HTMLInputElement.cpp:
> +        (WebCore::HTMLInputElement::getAllowedValueStep):
> +        (WebCore):
> +        * html/HTMLInputElement.h:
> +        (WebCore):
> +        (HTMLInputElement):
> +        * html/InputType.cpp:
> +        (WebCore::InputType::getAllowedValueStep):
> +        (WebCore):
> +        * html/InputType.h:
> +        (WebCore):
> +        (InputType):
> +        * html/RangeInputType.cpp:
> +        (WebCore::RangeInputType::getAllowedValueStep):
> +        (WebCore):
> +        (WebCore::RangeInputType::handleKeydownEvent):
> +        (WebCore::RangeInputType::fallbackValue):
> +        (WebCore::RangeInputType::maximum):
> +        (WebCore::RangeInputType::minimum):
> +        (WebCore::RangeInputType::sanitizeValue):
> +        * html/RangeInputType.h:
> +        (RangeInputType):
> +        * html/StepRange.cpp:
> +        (WebCore::StepRange::StepRange):
> +        (WebCore::StepRange::clampValue):
> +        (WebCore::StepRange::valueFromElement):
> +        * html/StepRange.h:
> +        (StepRange):
> +        (WebCore::StepRange::defaultValue):
> +        (WebCore::StepRange::proportionFromValue):
> +        (WebCore::StepRange::valueFromProportion):
> +        * html/shadow/SliderThumbElement.cpp:
> +        (WebCore::sliderPosition):
> +        (WebCore::RenderSliderThumb::layout):
> +        (WebCore::SliderThumbElement::setPositionFromPoint):

Please write what you changed / why you changed, especially for existing functions/files.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:20206
> +				458A2810154AB5FB00004037 /* Decimal.h */,
> +				458A280E154AB5E600004037 /* Decimal.cpp */,
>  				49E912A40EFAC8E6009D0CAF /* animation */,
>  				FD31604012B026A300C1A359 /* audio */,

I recommend inserting new files to sorted positions in order to avoid patch conflict.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:27862
>  				78D02BC5154A18DF00B62D05 /* CSSPropertyAnimation.cpp in Sources */,
> +				458A280F154AB5E600004037 /* Decimal.cpp in Sources */,

ditto.

> Source/WebCore/html/RangeInputType.cpp:114
>  double RangeInputType::minimum() const
>  {
> -    return parseToDouble(element()->fastGetAttribute(minAttr), rangeDefaultMinimum);
> +    return parseToDouble(StepRange(element()).minimum.toString(), numeric_limits<double>::quiet_NaN());
>  }
>  
>  double RangeInputType::maximum() const

Do we need these functions?

> Source/WebCore/html/RangeInputType.cpp:202
> +    const Decimal bigStep = ::std::max((stepRange.maximum - stepRange.minimum) / 10, step);

::std::max() should be max().

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:76
> +    if (!parseToDoubleForNumberType(string, &doubleResult))
> +        return false;
> +    *result = Decimal::fromString(string);

Parsing twice is not reasonable.

> Source/WebCore/platform/Decimal.cpp:353
> +Decimal::Decimal(uint64_t coefficient, int exponent, int sign)
> +    : m_data(coefficient, exponent, sign)

Passing sign as int is unreasonable.  It should be an enum.

> Source/WebCore/platform/Decimal.h:120
> +    static Decimal quietNaN(int);

Do we need to support negative NaN?

> Source/WebKit/chromium/tests/DecimalTest.cpp:113
> +TEST_F(DecimalTest, AppNumberStepUpStepDownFromRenderer)

What's 'App'?

> Source/WebKit/chromium/tests/DecimalTest.cpp:195
> +}

Please add more fromString tests.  e.g.
fromString(".5")
fromString(" 123 ")
fromString("1,234")
fromString("+3")
fromString("INF")
...

> Source/WebKit/chromium/tests/DecimalTest.cpp:216
> +    const Decimal inf(Decimal::infinity(0));
> +    const Decimal minf(Decimal::infinity(1));
> +    const Decimal nan(Decimal::quietNaN(0));
> +    const Decimal mnan(Decimal::quietNaN(1));
> +    const Decimal ten(10);

They should be:
Infinity
MinusInfinity / NegativeInfinity
NaN
MinusNaN / NegativeNaN
Ten

> Source/WebKit/chromium/tests/DecimalTest.cpp:255
> +}

We should add tests for values with larger/smaller exponent.

> Source/WebKit/chromium/tests/DecimalTest.cpp:329
> +}

We should add tests for values with larger/smaller exponent.

[Kent Tamura]

Comment on attachment 139585 [details]
Patch 5

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

> Source/WebKit/chromium/tests/DecimalTest.cpp:117
> +    EXPECT_EQ(String("1"), stepUp("0", "1", "0.33333333333333333", "0", 3)); // step=1/3

We should have a similar test:
  EXPECT_EQ(String("0.01"), stepUp("0", "0.01", "0.0033333333333333333", "0", 3)); // step=1/300

> Source/WebKit/chromium/tests/DecimalTest.cpp:156
> +}

We should have a test for Decimal(-2147483648).

[Kent Tamura]

Comment on attachment 139585 [details]
Patch 5

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

> Source/WebCore/platform/Decimal.cpp:48
> +static const int PrecisionOfExponentBits = 11;
> +static const int PrecisionOfCoefficientBits = 64;

These are not used.

> Source/WebCore/platform/Decimal.cpp:49
> +static const uint64_t MaxCoefficient = 0x16345785D89FFFFull; // 999999999999999999; // 18 9's

I'm afraid this is not compilable with Visual Studio 2008.
See the discussion in Bug 64522.

Created attachment 143523 [details]
Patch 6 - For Makefile validation

[Early Warning System Bot]

Comment on attachment 143523 [details]
Patch 6 - For Makefile validation

Attachment 143523 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12773193

[Early Warning System Bot]

Comment on attachment 143523 [details]
Patch 6 - For Makefile validation

Attachment 143523 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12779154

[Build Bot]

Comment on attachment 143523 [details]
Patch 6 - For Makefile validation

Attachment 143523 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12767262

[WebKit Review Bot]

Comment on attachment 143523 [details]
Patch 6 - For Makefile validation

Attachment 143523 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12780173

[Build Bot]

Comment on attachment 143523 [details]
Patch 6 - For Makefile validation

Attachment 143523 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12778177

Created attachment 143716 [details]
Patch 7 - For Makefile validation

[Build Bot]

Comment on attachment 143716 [details]
Patch 7 - For Makefile validation

Attachment 143716 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12804017

[Early Warning System Bot]

Comment on attachment 143716 [details]
Patch 7 - For Makefile validation

Attachment 143716 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12805015

[Early Warning System Bot]

Comment on attachment 143716 [details]
Patch 7 - For Makefile validation

Attachment 143716 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12791082

Created attachment 143719 [details]
Patch 8 - For Makefile validation

[WebKit Review Bot]

Comment on attachment 143719 [details]
Patch 8 - For Makefile validation

Attachment 143719 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12798055

Created attachment 143753 [details]
Patch 9

Comment on attachment 143753 [details]
Patch 9

* Build with CR-Linux and CR-Mac
* Apply Decimal to Date/DateTime/DateTimeLocal/Month/Time/Week in addition to Number/Range.

Could you review again?
Thanks in advance.

[Kent Tamura]

Comment on attachment 143753 [details]
Patch 9

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

I agree with this strategy, but the patch is too large. 197KB code change in one patch is not acceptable.

Please make another patch with only Decimal.h, Decimal.cpp, and DecimalTest.cpp as the first step.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:92
> +    // FIXME: UINT64_C(17976931348623159) should be used but it did not compile on Qt bots.
> +#if COMPILER(MSVC)
> +    const uint64_t leadingDigitsOf2Power1024 = 17976931348623159ui64;
> +#else
> +    const uint64_t leadingDigitsOf2Power1024 = 17976931348623159ull;
> +#endif

You use this workaround many times.  We should fix this issue before this patch.
I think we can add UINT64_C in wtf/MathExtras.h.

> LayoutTests/fast/forms/range/range-value-rounding-expected.txt:119
> +PASS createSample("0.000001", "0.000009", "1e-5").valueAsNumber is 0.000009
> +FAIL createSample("0.0000001", "0.0000001", "1e-6").value should be 0.0000001. Was 1e-7.
> +PASS createSample("0.0000001", "0.0000001", "1e-6").valueAsNumber is 0.0000001
> +FAIL createSample("0.0000001", "0.0000002", "1e-6").value should be 0.0000002. Was 2e-7.
> +PASS createSample("0.0000001", "0.0000002", "1e-6").valueAsNumber is 0.0000002
> +FAIL createSample("0.0000001", "0.0000003", "1e-6").value should be 0.0000003. Was 3e-7.

This has a lot of FAIL lines.

[Nikolas Zimmermann]

(In reply to comment #6)
> Until all compilers support decimal128, we should have third party one or implemented by ourselves, not full version.

Why don't you just ensure that 0.6000000000001 shows up as 0.6 instead of adding a whole new Decimal class, just to avoid some sanitizing? I'm not sure I follow your strategy here.

Created attachment 145264 [details]
Patch 10

[Early Warning System Bot]

Comment on attachment 145264 [details]
Patch 10

Attachment 145264 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12873237

[Early Warning System Bot]

Comment on attachment 145264 [details]
Patch 10

Attachment 145264 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12875043

[Build Bot]

Comment on attachment 145264 [details]
Patch 10

Attachment 145264 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12863494

[WebKit Review Bot]

Comment on attachment 145264 [details]
Patch 10

Attachment 145264 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12863555

Created attachment 145498 [details]
Patch 11

Comment on attachment 145498 [details]
Patch 11

Could you review this patch?

Note: This patch will be landed after bug 88208 (can't enter "0" to number input type) with rebase.

[Kent Tamura]

Comment on attachment 145498 [details]
Patch 11

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

> Source/WebCore/ChangeLog:1
> [phone]  Yoshifumi Inoue  <[e-mail]>

This patch is still too large to be reviewed.
We should have one or more preparation patches.

I'd like to proceed as follows:
 1. Introduce InputNumber type as an alias of double, and replace double with InputNumber or const InputNumber& in *InputType.{cpp,h}.
 2. Actual behavior change patch.  This makes InputNumber an alias of Decimal.
 3. Replace InputNumber with Decimal.


The next patch would be a preparation of step 1.
 Rename parseToDouble() to parse().
 Rename some doubleValue to numericValue or value
 Replace "static const double" with "static const int" in *InputType.cpp.

After that, we should introduce InputNumber, convertDoubleToInputNumber(), convertInputNumberToDouble(), and replace function signature to use InputNumber/const InputNumber&.

> Source/WebCore/html/BaseDateAndTimeInputType.cpp:57
> -    return parseToDouble(element()->value(), DateComponents::invalidMilliseconds());
> +    return convertDecimalToDouble(parseToDecimal(element()->value(), convertDoubleToDecimal(DateComponents::invalidMilliseconds())));
>  }

I don't think this code works because DateComponents::invalidMilliseconds() is NaN and Decimal::fromString() doesn't support NaN.

This code is inefficient. How about keeping parseToDouble() as a static function?

> Source/WebCore/html/BaseDateAndTimeInputType.cpp:184
>      if (!isfinite(parsedValue))
>          return visibleValue;
>  
> -    return serializeWithMilliseconds(parsedValue);
> +    const Decimal value = convertDoubleToDecimal(parsedValue);
> +    return value.isFinite() ? serializeWithMilliseconds(value) : visibleValue;

Do we need to check isFinite() again?

> Source/WebCore/html/parser/HTMLParserIdioms.h:74
> +inline double convertDecimalToDouble(const Decimal& value)
> +{
> +    return parseToDoubleForNumberType(serializeForNumberType(value));
> +}
> +
> +inline Decimal convertDoubleToDecimal(double value)
> +{
> +    return parseToDecimalForNumberType(serializeForNumberType(value));
> +}

They shouldn't be here because they're unrelated to HTML parsing.
We should move them to Decimal.{cpp,h}.  We don't need to use parseToDoubleForNumberType() and serializeForNumberType(double) to implement them.

> LayoutTests/fast/forms/range/range-value-rounding-expected.txt:549
> +0.1
> +0.1
> +0.2
> +0.2
> +0.3
> +0.3
> +0.4
> +0.4
> +0.5
> +0.5
> +0.6
> +0.6
> +0.7
> +0.7
> +0.8
> +0.8
> +0.9
> +0.9
> +0.01
> +0.01
> +0.02
> +0.02
> +0.03
> +0.03
> +0.04
> +0.04
> +0.05
> +0.05
> +0.06
> +0.06
> +0.07
> +0.07
> +0.08
> +0.08
> +0.09
> +0.09
> +0.001
> +0.001
> +0.002
> +0.002
> +0.003
> +0.003
> +0.004
> +0.004
> +0.005
> +0.005
> +0.006
> +0.006
> +0.007
> +0.007
> +0.008
> +0.008
> +0.009
> +0.009
> +0.0001
> +0.0001
> +0.0002
> +0.0002
> +0.0003
> +0.0003
> +0.0004
> +0.0004
> +0.0005
> +0.0005
> +0.0006
> +0.0006
> +0.0007
> +0.0007
> +0.0008
> +0.0008
> +0.0009
> +0.0009
> +0.00001
> +0.00001
> +0.00002
> +0.00002
> +0.00003
> +0.00003
> +0.00004
> +0.00004
> +0.00005
> +0.00005
> +0.00006
> +0.00006
> +0.00007
> +0.00007
> +0.00008
> +0.00008
> +0.00009
> +0.00009
> +0.000001
> +0.000001
> +0.000002
> +0.000002
> +0.000003
> +0.000003
> +0.000004
> +0.000004
> +0.000005
> +0.000005
> +0.000006
> +0.000006
> +0.000007
> +0.000007
> +0.000008
> +0.000008
> +0.000009
> +0.000009
> +0.0000001
> +0.0000001
> +0.0000002
> +0.0000002
> +0.0000003
> +0.0000003
> +0.0000004
> +0.0000004
> +0.0000005
> +0.0000005
> +0.0000006
> +0.0000006
> +0.0000007
> +0.0000007
> +0.0000008
> +0.0000008
> +0.0000009
> +0.0000009
> +0.00000001
> +0.00000001
> +0.00000002
> +0.00000002
> +0.00000003
> +0.00000003
> +0.00000004
> +0.00000004
> +0.00000005
> +0.00000005
> +0.00000006
> +0.00000006
> +0.00000007
> +0.00000007
> +0.00000008
> +0.00000008
> +0.00000009
> +0.00000009
> +0.000000001
> +0.000000001
> +0.000000002
> +0.000000002
> +0.000000003
> +0.000000003
> +0.000000004
> +0.000000004
> +0.000000005
> +0.000000005
> +0.000000006
> +0.000000006
> +0.000000007
> +0.000000007
> +0.000000008
> +0.000000008
> +0.000000009
> +0.000000009
> +0.0000000001
> +0.0000000001
> +0.0000000002
> +0.0000000002
> +0.0000000003
> +0.0000000003
> +0.0000000004
> +0.0000000004
> +0.0000000005
> +0.0000000005
> +0.0000000006
> +0.0000000006
> +0.0000000007
> +0.0000000007
> +0.0000000008
> +0.0000000008
> +0.0000000009
> +0.0000000009
> +0.00000000001
> +0.00000000001
> +0.00000000002
> +0.00000000002
> +0.00000000003
> +0.00000000003
> +0.00000000004
> +0.00000000004
> +0.00000000005
> +0.00000000005
> +0.00000000006
> +0.00000000006
> +0.00000000007
> +0.00000000007
> +0.00000000008
> +0.00000000008
> +0.00000000009
> +0.00000000009
> +0.000000000001
> +0.000000000001
> +0.000000000002
> +0.000000000002
> +0.000000000003
> +0.000000000003
> +0.000000000004
> +0.000000000004
> +0.000000000005
> +0.000000000005
> +0.000000000006
> +0.000000000006
> +0.000000000007
> +0.000000000007
> +0.000000000008
> +0.000000000008
> +0.000000000009
> +0.000000000009
> +0.0000000000001
> +0.0000000000001
> +0.0000000000002
> +0.0000000000002
> +0.0000000000003
> +0.0000000000003
> +0.0000000000004
> +0.0000000000004
> +0.0000000000005
> +0.0000000000005
> +0.0000000000006
> +0.0000000000006
> +0.0000000000007
> +0.0000000000007
> +0.0000000000008
> +0.0000000000008
> +0.0000000000009
> +0.0000000000009
> +0.00000000000001
> +0.00000000000001
> +0.00000000000002
> +0.00000000000002
> +0.00000000000003
> +0.00000000000003
> +0.00000000000004
> +0.00000000000004
> +0.00000000000005
> +0.00000000000005
> +0.00000000000006
> +0.00000000000006
> +0.00000000000007
> +0.00000000000007
> +0.00000000000008
> +0.00000000000008
> +0.00000000000009
> +0.00000000000009
> +0.000000000000001
> +0.000000000000001
> +0.000000000000002
> +0.000000000000002
> +0.000000000000003
> +0.000000000000003
> +0.000000000000004
> +0.000000000000004
> +0.000000000000005
> +0.000000000000005
> +0.000000000000006
> +0.000000000000006
> +0.000000000000007
> +0.000000000000007
> +0.000000000000008
> +0.000000000000008
> +0.000000000000009
> +0.000000000000009

We had better remove these useless strings.

[Kent Tamura]

(In reply to comment #42)
> (In reply to comment #6)
> > Until all compilers support decimal128, we should have third party one or implemented by ourselves, not full version.
> 
> Why don't you just ensure that 0.6000000000001 shows up as 0.6 instead of adding a whole new Decimal class, just to avoid some sanitizing? I'm not sure I follow your strategy here.

The current code is trying to resolve such issues by rounding and passing 'decimalPlaces' information. But
 - We need complex code to keep track decimalPlaces correctly, and one can easily forget to apply decimalPlaces.
 - decimalPlaces is not perfect. We still have bugs in the code with decimalPlaces.
 - The current code needs some precision loss because of rounding and decimalPlaces. We need to support double precision according to the standard.

If we apply decimal arithmetic, the implementation would be really simple.

All blocking patches has been landed.

* Behavior change was started at r119948 (Jun 10 2012 19:33 PST)
* Source code cleanup was done at r120313 (Jun 14 2012, 04:54 PST)

Thanks for waiting about two month with patient.

This bug is also reported as http://crbug.com/125500