[webkit-reviews] review granted: [Bug 36206] Fix the rendering of HTMLProgressElement : [Attachment 51738] Patch v3

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Yael <yael.aharon at nokia.com>'s
request for review:
Bug 36206: Fix the rendering of HTMLProgressElement
https://bugs.webkit.org/show_bug.cgi?id=36206

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
r=me, but consider changing these:

> Index: WebCore/platform/qt/RenderThemeQt.cpp
> ===================================================================
> --- WebCore/platform/qt/RenderThemeQt.cpp	(revision 56617)
> +++ WebCore/platform/qt/RenderThemeQt.cpp	(working copy)
> @@ -679,7 +679,7 @@ bool RenderThemeQt::paintProgressBar(Ren
>      option.rect = r;
>      option.maximum = 65536;
>      option.minimum = 0;
> -    option.progress = renderProgress->position();
> +    option.progress = (renderProgress->position() * 65536);

Where does the magic 65536 come from? Please use
std::numeric_limits<int>::max() or similar.

> @@ -97,21 +57,12 @@ void RenderProgress::layout()
>  void RenderProgress::updateFromElement()
>  {
>      HTMLProgressElement* element =
static_cast<HTMLProgressElement*>(node());
> -    double position = element->position();
> -    int oldPosition = m_position;
> -    bool canOptimize =
theme()->getNumberOfPixelsForProgressPosition(position, m_position);
> +    double oldPosition = m_position;
> +    m_position = element->position();
>      if (oldPosition == m_position)
>	   return;

Could you just do if (m_position == element->position()). oldPosition does not
seem to be used anywhere else.
yea