Fix bugs related to setting reflected floating point DOM attributes
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Oct 2017 22:17:46 +0000 (22:17 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Oct 2017 22:17:46 +0000 (22:17 +0000)
commit6cb5ed357c61c77769eecff0120aa1a2b8e9626d
tree3e57eacc789ecdd6d6ccb0eab1835a0aeb70b8c1
parent0e60f236595048fded42d8f89cf076cea93aed84
Fix bugs related to setting reflected floating point DOM attributes
https://bugs.webkit.org/show_bug.cgi?id=178061

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

* web-platform-tests/html/dom/reflection-forms-expected.txt:
Updated to expect meter reflection tests to pass.

Source/WebCore:

* html/HTMLProgressElement.cpp:
(WebCore::HTMLProgressElement::setValue): Changed the semantics to match what
the HTML specification calls for. When a caller passes a negative number or
zero, the value does get set on the element. Negative numbers are not allowed
when you get the current value, but are allowed to be set.
(WebCore::HTMLProgressElement::setMax): Changed the semantics to match what
the HTML specification calls for. When a caller passes a negative number or
zero, this should leave the attribute unchanged.

* html/shadow/MediaControlElementTypes.cpp:
(WebCore::MediaControlVolumeSliderElement::setVolume): Use
String::numberToStringECMAScript instead of String::number since that is what
we want any time we are setting an attribute value from a floating point value.
* html/shadow/MediaControlElements.cpp:
(WebCore::MediaControlTimelineElement::setPosition): Ditto.
(WebCore::MediaControlTimelineElement::setDuration): Removed unneeded check
of std::isfinite since the single caller already checks that.

Source/WTF:

* wtf/dtoa.cpp:
(WTF::formatStringTruncatingTrailingZerosIfNeeded): Fix a bug where
this function would remove trailing zeroes from the exponent if
present instead of from the mantissa. This meant that it would
format 1e10 as "1.00000e+1" instead of "1e+10". Added regression
tests for this to TestWebKitAPI.

* wtf/dtoa/utils.h:
(WTF::double_conversion::StringBuilder::RemoveCharacters): Added.
Used by the fix above.

* wtf/text/AtomicString.cpp:
(WTF::AtomicString::number): Note: This function is used by code
that sets the values of reflected floating point DOM attributes.
Changed the function to use the rules from numberToString rather
ones from numberToFixedPrecisionString. This is analogous to
String::numberToStringECMAScript, and in the future we should change
String and StringBuilder so this "ECMAScript" semantic is the default
way to convert a floating point number to a string, and rename the fixed
precision version that is currently called String::number. I audited
the small number of sites calling AtomicString::number, by temporarily
renaming it, and discovered that this is the correct behavior for all;
none needed fixed precision. Also, fixed a mistake where this explicitly
converted to String. That was defeating the purpose of having these
functions in AtomicString: It would allocate a new String and then
destroy it in the case where an equal string was already in the
AtomicString table.

Tools:

* TestWebKitAPI/Tests/WTF/AtomicString.cpp: Added a test of the
AtomicString::number function, based on the test cases we already
had for String::numberToStringECMAScript, and with some additional
cases with powers of 10 that check handling of trailng zeroes.

* TestWebKitAPI/Tests/WTF/WTFString.cpp: Added test cases to the
existing tests of the String::numberToStringECMAScript function
as above. Also added tests for String::number and for
String::numberToStringFixedWidth. Also changed the tests to all use
EXPECT instead of ASSERT macros since these are all non-fatal.

* WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
(WTR::dumpFrameScrollPosition): Use StringBuilder::appendECMAScriptNumber
instead of String::number.

LayoutTests:

* fast/dom/HTMLProgressElement/set-progress-properties-expected.txt: Updated test to expect
setting HTMLProgressElement.max to 0 to have no effect, rather than setting max to "1".
* fast/dom/HTMLProgressElement/set-progress-properties.html: Ditto.

* platform/ios-wk2/imported/w3c/web-platform-tests/html/dom/reflection-forms-expected.txt:
Updated to expect meter reflection tests to pass.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223035 268f45cc-cd09-0410-ab3c-d52691b4dbfc
18 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLProgressElement/set-progress-properties-expected.txt
LayoutTests/fast/dom/HTMLProgressElement/set-progress-properties.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/dom/reflection-forms-expected.txt
LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/dom/reflection-forms-expected.txt
Source/WTF/ChangeLog
Source/WTF/wtf/dtoa.cpp
Source/WTF/wtf/dtoa/utils.h
Source/WTF/wtf/text/AtomicString.cpp
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLProgressElement.cpp
Source/WebCore/html/shadow/MediaControlElementTypes.cpp
Source/WebCore/html/shadow/MediaControlElements.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/AtomicString.cpp
Tools/TestWebKitAPI/Tests/WTF/WTFString.cpp
Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp