[Webkit-unassigned] [Bug 39370] [chromium] Fix slider status when buffering

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56511|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #3 from David Levin <levin at chromium.org>  2010-05-19 16:19:03 PST ---
(From update of attachment 56511)
Thanks Alpha for looking it over.

Just a few minor nits to address.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog

> +
> +        * rendering/RenderMediaControlsChromium.cpp:
> +        (WebCore::paintMediaSlider):
> +        Added logic to align the buffering bar with the thumb. Half of the thumb image is transparent, so the buffer bar is adjusted to fill in this gap.

ChangeLog entries tend to wrap text somewhere around 80 columns.


> diff --git a/WebCore/rendering/RenderMediaControlsChromium.cpp b/WebCore/rendering/RenderMediaControlsChromium.cpp
> +static Image* getMediaSliderThumb()
> +{
> +    static Image* mediaSliderThumb  = platformResource("mediaSliderThumb");

Two spaces before =

> +
> +    double bufferedWidth = 0.0;
> +    if (mediaElement->percentLoaded() > 0.0) {
> +        // Account for width of slider thumb

Please add a period to the end of the comment.

> +        Image* mediaSliderThumb = getMediaSliderThumb();
> +        double thumbWidth = mediaSliderThumb->width() / 2.0 + 1.0;
> +        double rectWidth = bufferedRect.width() - thumbWidth;
> +        if (rectWidth < 0)
> +            rectWidth = 0;

I'm not sure why you initialized "double bufferedWidth" with 0.0 above but are using 0 here. (It doesn't matter but inconsistency raises flags for me.)

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