Bug 195253 – Retire legacy dtoa function and DecimalNumber class

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Darin Adler]

Retire legacy dtoa function and DecimalNumber class

[Darin Adler]

Created attachment 363438 [details]
Patch

[Darin Adler]

Created attachment 363439 [details]
Patch

[EWS Watchlist]

Comment on attachment 363439 [details]
Patch

Attachment 363439 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11348091

New failing tests:
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html
media/modern-media-controls/macos-inline-media-controls/macos-inline-media-controls-volume-styles.html
imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html
fast/css/large-value-csstext.html

[EWS Watchlist]

Created attachment 363441 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363439 [details]
Patch

Attachment 363439 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11348117

New failing tests:
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html
media/modern-media-controls/macos-inline-media-controls/macos-inline-media-controls-volume-styles.html
imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html
fast/css/large-value-csstext.html

[EWS Watchlist]

Created attachment 363442 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363439 [details]
Patch

Attachment 363439 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11348165

New failing tests:
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html
media/modern-media-controls/macos-inline-media-controls/macos-inline-media-controls-volume-styles.html
fast/css/large-value-csstext.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation.html

[EWS Watchlist]

Created attachment 363443 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363439 [details]
Patch

Attachment 363439 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11348211

New failing tests:
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html
fast/css/large-value-csstext.html

[EWS Watchlist]

Created attachment 363444 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 363439 [details]
Patch

Attachment 363439 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11348379

New failing tests:
fast/css/large-value-csstext.html

[EWS Watchlist]

Created attachment 363445 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Darin Adler]

Created attachment 363448 [details]
Patch

[Darin Adler]

Created attachment 363449 [details]
Patch

[EWS Watchlist]

Comment on attachment 363449 [details]
Patch

Attachment 363449 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11350326

New failing tests:
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html
imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html

[EWS Watchlist]

Created attachment 363453 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Darin Adler]

Created attachment 363457 [details]
Patch

[Darin Adler]

Passes all tests, ready for review and landing. Lets us delete a lot of code!

[Daniel Bates]

Comment on attachment 363457 [details]
Patch

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

> Source/WTF/wtf/JSONValues.cpp:677
> +        else
> +            output.appendECMAScriptNumber(m_value.number);

Nice.

> Source/WebCore/css/CSSPrimitiveValue.cpp:1015
> +        return "attr(" + String(m_value.string) + ')';

Ok as-is. Could write using uniform initializer syntax.

> LayoutTests/fast/css/large-value-csstext-expected.txt:4
> -100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000%
> -0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009%
> +1e+254%
> +9e-249%

What nonsense we were emitting. Have to ask though, are these results compatible with other browsers? Closer to other browsers? Farther from?

[Darin Adler]

Comment on attachment 363457 [details]
Patch

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

>> LayoutTests/fast/css/large-value-csstext-expected.txt:4
>> +9e-249%
> 
> What nonsense we were emitting. Have to ask though, are these results compatible with other browsers? Closer to other browsers? Farther from?

Someone could investigate, I prefer not to at this time. This is currently the *only* test affected. Not sure there are *practical* considerations with enormous values. CSS serialization of floating point numbers is likely quite inconsistent already due to things like mixing single and double precision so there is a ton of room for improvement and I don’t think we’re truly ready to standardize these to help boost compatibility.

[Darin Adler]

Comment on attachment 363457 [details]
Patch

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

>> Source/WebCore/css/CSSPrimitiveValue.cpp:1015
>> +        return "attr(" + String(m_value.string) + ')';
> 
> Ok as-is. Could write using uniform initializer syntax.

I chose to match the style of the thing down one line from here. Didn’t think too deeply about the best syntax.

[Darin Adler]

Committed r242330: <https://trac.webkit.org/changeset/242330>

[Radar WebKit Bug Importer]

<rdar://problem/48545602>

[Simon Fraser (smfr)]

Comment on attachment 363457 [details]
Patch

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

>>> LayoutTests/fast/css/large-value-csstext-expected.txt:4
>>> +9e-249%
>> 
>> What nonsense we were emitting. Have to ask though, are these results compatible with other browsers? Closer to other browsers? Farther from?
> 
> Someone could investigate, I prefer not to at this time. This is currently the *only* test affected. Not sure there are *practical* considerations with enormous values. CSS serialization of floating point numbers is likely quite inconsistent already due to things like mixing single and double precision so there is a ton of room for improvement and I don’t think we’re truly ready to standardize these to help boost compatibility.

Luckily CSS does allow scientific notion in numeric values now, and we support it (that only changed a couple of years go). I am slightly uneasy with the compatibility risk of this change, although anyone using parseFloat() in JS on computed style should end up with the same result.

[Simon Fraser (smfr)]

(In reply to Simon Fraser (smfr) from comment #24)
> Comment on attachment 363457 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363457&action=review
> 
> >>> LayoutTests/fast/css/large-value-csstext-expected.txt:4
> >>> +9e-249%
> >> 
> >> What nonsense we were emitting. Have to ask though, are these results compatible with other browsers? Closer to other browsers? Farther from?
> > 
> > Someone could investigate, I prefer not to at this time. This is currently the *only* test affected. Not sure there are *practical* considerations with enormous values. CSS serialization of floating point numbers is likely quite inconsistent already due to things like mixing single and double precision so there is a ton of room for improvement and I don’t think we’re truly ready to standardize these to help boost compatibility.
> 
> Luckily CSS does allow scientific notion in numeric values now, and we
> support it (that only changed a couple of years go). I am slightly uneasy
> with the compatibility risk of this change, although anyone using
> parseFloat() in JS on computed style should end up with the same result.

Chrome uses scientific notation so we're probably OK.