Fix bugs related to setting reflected floating point DOM attributes
Created attachment 323129 [details] Patch
Created attachment 323130 [details] Patch
Created attachment 323131 [details] Patch
Comment on attachment 323131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323131&action=review > Source/WebCore/ChangeLog:13 > + (WebCore::HTMLProgressElement::setMax): Changed the semantics to match what May want to mention: https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#limited-to-numbers-greater-than-zero > Source/WebCore/html/HTMLProgressElement.cpp:101 > + setAttributeWithoutSynchronization(valueAttr, AtomicString::number(value)); The specification [1[ says to use the "best representation of the number as a floating-point number" [2]. It basically says to use Ecma's ToString(), so shouldn't it call String::numberToStringECMAScript() ? Same for meter.value. - https://html.spec.whatwg.org/multipage/form-elements.html#dom-progress-value - https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#best-representation-of-the-number-as-a-floating-point-number
Comment on attachment 323131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323131&action=review >> Source/WebCore/ChangeLog:13 >> + (WebCore::HTMLProgressElement::setMax): Changed the semantics to match what > > May want to mention: > https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#limited-to-numbers-greater-than-zero OK >> Source/WebCore/html/HTMLProgressElement.cpp:101 >> + setAttributeWithoutSynchronization(valueAttr, AtomicString::number(value)); > > The specification [1[ says to use the "best representation of the number as a floating-point number" [2]. It basically says to use Ecma's ToString(), so shouldn't it call String::numberToStringECMAScript() ? > Same for meter.value. > > - https://html.spec.whatwg.org/multipage/form-elements.html#dom-progress-value > - https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#best-representation-of-the-number-as-a-floating-point-number Yes, exactly. AtomicSstring::number does exactly the same thing as String::numberToStringECMAScript. The String::number function needs to be renamed to include the words “fixed precision” and we will not use it that much any more going forward. I am trying to make the ECMAScript behavior, which is a great default anyway, the default stringifying for doubles throughout WebKit over time. So yes, this is what this patch does and that’s why the tests are passing now. I could have renamed the AtomicStriing::number function to add the word ECMAScript, but then I would’ve had to change all the call sites, and all of them want the ECMAScript semantic. I chose to do it this way: it’s a bit more subtle but I think it’s good forward-looking.
Maybe I should add more tests; not sure every last behavior change is sufficiently covered.
Now that EWS has run: review anyone?
Comment on attachment 323131 [details] Patch Clearing flags on attachment: 323131 Committed r223035: <http://trac.webkit.org/changeset/223035>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34877132>