[Webkit-unassigned] [Bug 37308] [Chromium] Support HTML5 <progress> element on Windows.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=37308


Shinichiro Hamaji <hamaji at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #53905|review?                     |review-
               Flag|                            |




--- Comment #9 from Shinichiro Hamaji <hamaji at chromium.org>  2010-04-20 19:03:17 PST ---
(From update of attachment 53905)
> +double RenderThemeChromiumWin::animationDurationForProgressBar(RenderProgress* o) const

I'm not sure if the name 'o' is good for an object of RenderProgress. I would
say renderProgress or something like this.

> +    if (o->position() < 0)

I couldn't understand why this is necessary. If this is necessary, don't we
need to modfify RenderThemeMac as well?

Also, it would be nice to add a function which returns position()<0 in
RenderProgress (maybe isIndeterminate or something like this?). Anyway, I guess
we want some refactoring around this patch, but in a separated patch.

> +    int fillPart;
> +    IntRect fillRect;
> +
> +

Unnecessary blank line(s). I guess we need no blanklines here.

> +        int dx = r.width() * renderProgress->animationProgress() * 2.0 - r.width();

Maybe we should use 2 instead of 2.0. (See Floating point literals in
http://webkit.org/coding/coding-style.html)

r- for Tamura-san's comment and style issues.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.