[webkit-reviews] review denied: [Bug 28310] Chromium: Show a "Mute Disabled" button on audio error. : [Attachment 34864] Fix
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 14 15:14:18 PDT 2009
Eric Seidel <eric at webkit.org> has denied Kyle Prete <kylep at chromium.org>'s
request for review:
Bug 28310: Chromium: Show a "Mute Disabled" button on audio error.
https://bugs.webkit.org/show_bug.cgi?id=28310
Attachment 34864: Fix
https://bugs.webkit.org/attachment.cgi?id=34864&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
I'm not a media expert, but this patch looks wrong. Mostly because the methods
look out of place:
One of these does not look like the other:
virtual bool isVideo() const { return false; }
virtual bool hasVideo() const { return false; }
+ virtual bool hasAudio() const { return player() && player()->hasAudio(); }
And again:
virtual bool hasVideo() const = 0;
+ virtual bool hasAudio() const { return true; }
It seems maybe you're not following the design of the rest of the media
classes?
Please explain.
More information about the webkit-reviews
mailing list