Summary: | [chromium] Fix slider status when buffering | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Victoria Kirst <vrk> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, hclam, levin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Victoria Kirst
2010-05-19 11:52:25 PDT
Created attachment 56511 [details]
Patch
Victoria is working with me on HTML5 video in Chromium. This change looks good to me. 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.) Created attachment 56604 [details]
Patch
Comment on attachment 56604 [details] Patch Clearing flags on attachment: 56604 Committed r59932: <http://trac.webkit.org/changeset/59932> All reviewed patches have been landed. Closing bug. |