[webkit-reviews] review granted: [Bug 41718] Mac OS X media controls should have a way to adjust volume incrementally : [Attachment 60661] proposed patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Eric Carlson <eric.carlson at apple.com>'s request for
review:
Bug 41718: Mac OS X media controls should have a way to adjust volume
incrementally
https://bugs.webkit.org/show_bug.cgi?id=41718

Attachment 60661: proposed patch
https://bugs.webkit.org/attachment.cgi?id=60661&action=review

------- Additional Comments from mitz at webkit.org
> +	    
(WebCore::MediaControlMuteButtonElement::MediaControlMuteButtonElement): Add
ButtonLocation 
> +	     parameter both mute buttons can be created.

Missing a word there?


> +audio::-webkit-media-controls-volume-slider-container,
video::-webkit-media-controls-volume-slider-container {
> +    -webkit-appearance: media-volume-slider-container;
> +    position: absolute;
> +
> +    width: 22px;
> +    height: 114px;
> +}

I wonder if this can specify one of top/bottom and one of left/right.

> +IntPoint RenderThemeMac::volumeSliderOffsetFromMuteButton(Node* muteButton,
const IntSize& size) const
> +{
> +    static const int xOffset = -4;
> +    static const int yOffset = 5;
> +
> +    int y = muteButton->renderBox()->offsetHeight() + yOffset -
size.height();
> +    FloatPoint absPoint =
muteButton->renderer()->localToAbsolute(FloatPoint(muteButton->renderBox()->off
setLeft(), y), true, true);
> +    if (absPoint.y() < 0)
> +	   y = muteButton->renderBox()->height();
> +    return IntPoint(xOffset, y);
> +}

I think I know what this does but a comment wouldn’t hurt.

r=me!