[webkit-reviews] review denied: [Bug 28196] Chromium: Show a "Playback Disabled" button on media error. : [Attachment 34608] Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 11 16:25:08 PDT 2009

David Levin <levin at chromium.org> has denied Kyle Prete <kylep at chromium.org>'s
request for review:
Bug 28196: Chromium: Show a "Playback Disabled" button on media error.

Attachment 34608: Fix

------- Additional Comments from David Levin <levin at chromium.org>
The diff here is odd.  It should have WebCore/ prefixed before each file.  I
suspect that you are working from a chromium directory and that's why you have
this problem.  (I don't do that so I don't know the fix.)  At worst fix it by
hand to aid in landing your patch.

> Index: ChangeLog
> +2009-08-11  Kyle Prete  <kylep at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +

You need the bug title and a link to the bug here. (see other changelog

prepare-ChangeLog -bug # will give the you the right format.

> +	   Use a disabled play button when the media file cannot be played.
> +
> +	   No new tests. (OOPS!)

This shouldn't be here.  Either add a test or indicate if existing tests cover

> Index: rendering/RenderThemeChromiumSkia.cpp
>      static Image* mediaPlay =
>      static Image* mediaPause =
> +    static Image* mediaPlayDisabled =
> +    if (mediaElement->networkState() == HTMLMediaElement::NETWORK_NO_SOURCE)

> +	 return paintMediaButtonInternal(paintInfo.context, rect,

This needs a 4 space indent.

More information about the webkit-reviews mailing list