[Webkit-unassigned] [Bug 92132] Add an overlay play button to media controls on android

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=92132





--- Comment #2 from Eric Carlson <eric.carlson at apple.com>  2012-07-24 11:06:49 PST ---
(From update of attachment 154087)
View in context: https://bugs.webkit.org/attachment.cgi?id=154087&action=review

> Source/WebCore/css/mediaControls.css:86
> +    display: -webkit-box;

Shouldn't this be display:none, and then each port that wants to display it can override?

> Source/WebCore/html/shadow/MediaControlElements.h:283
> +    MediaControlOverlayPlayButtonElement(Document*);

All single-argument constructors should be marked "explicit" so the compiler doesn't try to implicitly create a MediaControlCromiumEnclosureElement from a Document*.

> Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp:78
> +#if ENABLE(MEDIA_OVERLAY_PLAY_BUTTON)
> +MediaControlOverlayEnclosureElement::MediaControlOverlayEnclosureElement(Document* document)
> +    : MediaControlChromiumEnclosureElement(document)
> +{
> +}
> +

Is there any reason to not do this by subclassing MediaControlRootElementChromium instead of adding compile flags?

> Source/WebCore/rendering/RenderMediaControlsChromium.cpp:116
> +#if ENABLE(MEDIA_OVERLAY_PLAY_BUTTON)
> +static bool paintMediaOverlayPlayButton(RenderObject* object, const PaintInfo& paintInfo, const IntRect& rect)
> +{

Again, is there any reason to not do this by subclassing?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.