Per the thread on bug 63861, progress element should be indeterminate when the value attribute is missing, per the spec here: <http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#the-progress-element> As is, HTMLProgressElement::isDeterminate tests whether the value is specified, and also < max.
It duplicates the logic for isDeterminate, uses slower attribute accessors than is necessary, and checks an impossible case (InvalidPosition) only used in its RenderProgress. I originally thought that this was buggy, however I don’t believe it is—just slightly obfuscated. Should HTMLProgressElement::InvalidPosition be deported to RenderProgress? That is the only place it is used, as a kind of flag to test whether the render object has been updated from the element for the first time or not.
Created attachment 99624 [details] Patch
(In reply to comment #1) > It duplicates the logic for isDeterminate, uses slower attribute accessors than is necessary, and checks an impossible case (InvalidPosition) only used in its RenderProgress. ok, it would be reasonable. > Should HTMLProgressElement::InvalidPosition be deported to RenderProgress? That is the only place it is used, as a kind of flag to test whether the render object has been updated from the element for the first time or not. I think InvalidPosition is not needed. we can initialize RenderProgress::m_position with element->position(). Morita-san, what do you think?
Comment on attachment 99624 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99624&action=review Looks good in general. have one question: > Source/WebCore/html/HTMLProgressElement.cpp:-140 > - return (HTMLProgressElement::IndeterminatePosition != currentPosition && HTMLProgressElement::InvalidPosition != currentPosition); Do we need InvalidPosition yet ?
Created attachment 99668 [details] Removes InvalidPosition
(In reply to comment #4) > Do we need InvalidPosition yet ? InvalidPosition was only used by RenderProgress; it initializes its position to InvalidPosition and then gets updated from the element. I have uploaded a patch that removes InvalidPosition entirely by initializing the position in the RenderProgress straight away… WDYT?
Comment on attachment 99668 [details] Removes InvalidPosition Initial InvalidPosition is still needed for starting indeterminate state animation.
Comment on attachment 99624 [details] Patch Clearing flags on attachment: 99624 Committed r90384: <http://trac.webkit.org/changeset/90384>
All reviewed patches have been landed. Closing bug.