[webkit-reviews] review denied: [Bug 63861] value DOM property returns 1 for indeterminate progress bars : [Attachment 99592] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Dominic Cooney
<dominicc at chromium.org>'s request for review:
Bug 63861: value DOM property returns 1 for indeterminate progress bars
https://bugs.webkit.org/show_bug.cgi?id=63861

Attachment 99592: Patch
https://bugs.webkit.org/attachment.cgi?id=99592&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99592&action=review

> Source/WebCore/html/HTMLProgressElement.cpp:100
> +    bool ok = parseToDoubleForNumberType(getAttribute(valueAttr), &value);

You had better use fastGetAttribute().

> Source/WebCore/html/HTMLProgressElement.cpp:108
> +    if (value > maxValue)
> +	   return maxValue;
> +
> +    return value;

No need to expand the original 1-line code to 4 lines.

> Source/WebCore/html/HTMLProgressElement.cpp:148
> +bool HTMLProgressElement::isDeterminate() const
>  {
> -    double currentPosition = position();
> -    return (HTMLProgressElement::IndeterminatePosition != currentPosition &&
HTMLProgressElement::InvalidPosition != currentPosition);
> +    return hasAttribute(valueAttr);

This is incorrect.
isDeterminate() should return true if fastHasAttribute(value) && the value is
<= the maximum value.