Bug 39370 - [chromium] Fix slider status when buffering
Summary: [chromium] Fix slider status when buffering
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-19 11:52 PDT by Victoria Kirst
Modified: 2010-05-21 07:44 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.98 KB, patch)
2010-05-19 12:40 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff
Patch (3.01 KB, patch)
2010-05-20 09:57 PDT, Victoria Kirst
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Victoria Kirst 2010-05-19 11:52:25 PDT
When you load a video, the slider shows the entire video has been buffered, when only a small portion of it has been buffered.
Comment 1 Victoria Kirst 2010-05-19 12:40:32 PDT
Created attachment 56511 [details]
Patch
Comment 2 Hin-Chung Lam 2010-05-19 12:45:47 PDT
Victoria is working with me on HTML5 video in Chromium. This change looks good to me.
Comment 3 David Levin 2010-05-19 16:19:03 PDT
Comment on attachment 56511 [details]
Patch

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.)
Comment 4 Victoria Kirst 2010-05-20 09:57:58 PDT
Created attachment 56604 [details]
Patch
Comment 5 WebKit Commit Bot 2010-05-21 07:44:18 PDT
Comment on attachment 56604 [details]
Patch

Clearing flags on attachment: 56604

Committed r59932: <http://trac.webkit.org/changeset/59932>
Comment 6 WebKit Commit Bot 2010-05-21 07:44:29 PDT
All reviewed patches have been landed.  Closing bug.