Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(328)

Issue 174119: Disable playback controls if audio/video element has errored. (Closed)

Created:
11 years, 4 months ago by scherkus (not reviewing)
Modified:
9 years, 7 months ago
Reviewers:
awong
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Disable playback controls if audio/video element has errored. TEST=load an audio/video with an unsupported input, then right click on it.. the playback control items should be disabled BUG=19740

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -1 line) Patch
M webkit/glue/context_menu_client_impl.cc View 3 chunks +4 lines, -1 line 3 comments Download

Messages

Total messages: 4 (0 generated)
scherkus (not reviewing)
11 years, 4 months ago (2009-08-20 01:55:00 UTC) #1
awong
http://codereview.chromium.org/174119/diff/1/2 File webkit/glue/context_menu_client_impl.cc (right): http://codereview.chromium.org/174119/diff/1/2#newcode21 Line 21: #include "MediaError.h" Do you need to include MediaError?
11 years, 4 months ago (2009-08-20 18:33:54 UTC) #2
scherkus (not reviewing)
http://codereview.chromium.org/174119/diff/1/2 File webkit/glue/context_menu_client_impl.cc (right): http://codereview.chromium.org/174119/diff/1/2#newcode21 Line 21: #include "MediaError.h" On 2009/08/20 18:33:54, awong wrote: > ...
11 years, 4 months ago (2009-08-20 18:50:24 UTC) #3
awong
11 years, 4 months ago (2009-08-20 18:57:52 UTC) #4
LGTM

http://codereview.chromium.org/174119/diff/1/2
File webkit/glue/context_menu_client_impl.cc (right):

http://codereview.chromium.org/174119/diff/1/2#newcode21
Line 21: #include "MediaError.h"
On 2009/08/20 18:50:24, scherkus wrote:
> On 2009/08/20 18:33:54, awong wrote:
> > Do you  need to include MediaError?
> 
> HTMLMediaElement::error() returns PassRefPtr<MediaError>.  The compiler dies
> when attempting to resolve that class.


Ah...makes sense.  <insert grumbling about making a template that requires a
fully defined type to do something that's not necessary>

Powered by Google App Engine
This is Rietveld 408576698