Bug 36206 – Fix the rendering of HTMLProgressElement

• Component: DOM
• Status: RESOLVED FIXED

[Yael]

I received today this link from Ian Hickson, which details the expected style for HTMLProgressElement.
http://www.whatwg.org/specs/web-apps/current-work/complete.html#the-progress-element-0 .

[Yael]

Created attachment 50932 [details]
Patch v1.

Update the style information to conform to http://www.whatwg.org/specs/web-apps/current-work/complete.html#the-progress-element-0.
Originally, I did copied the style of <input type="range">, not knowing that the style was actually specified.

Since this patch is blocked by https://bugs.webkit.org/show_bug.cgi?id=36113, and I am not sure how to handle this situation, I included the code that is up for review in https://bugs.webkit.org/show_bug.cgi?id=36113 in this patch.

[Yael]

Created attachment 50959 [details]
Patch v2.

Update the patch after https://bugs.webkit.org/show_bug.cgi?id=36113 was resolved.

[Yael]

This style adjustment will be merged to https://bugs.webkit.org/show_bug.cgi?id=36224.

[Yael]

Splitting up the patch from https://bugs.webkit.org/show_bug.cgi?id=36224 again.

[Yael]

Created attachment 51738 [details]
Patch v3

Previously, I thought that <progress> element should be simillar to the slider of <input type="range">, so I used code from RenderSlider, (baselinePosition and calcPrefWidths).
http://www.whatwg.org/specs/web-apps/current-work/complete.html#the-progress-element-0 specifies the rendering rules for <progress> element, so this patch fixes that mistake.
This patch also removes code in RenderProgress::updateFromElement() that tries to optimize the size of the rect to be repainted. The optimization will be redone later.

[Antti Koivisto]

Comment on attachment 51738 [details]
Patch v3

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

[Yael]

Landed in 56629