[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 #4 from Min Qin <qinmin at chromium.org>  2012-07-25 15:01:35 PST ---
Removed the ENABLE(MEDIA_OVERLAY_PLAY_BUTTON) flag, PTAL.

(In reply to comment #2)
> (From update of attachment 154087 [details])
> 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?

Done. 

> 
> > 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?

Refactored MediaControlRootElementChromium and added a subclass MediaControlRootElementChromiumAndroid to implement the functionality of  the overlay play button.

> 
> > 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?

This class is full of static functions. So subclassing it does not help too much. I removed the "#if ENABLE(MEDIA_OVERLAY_PLAY_BUTTON)" in the file, the code will only be hit in the android port (called from RenderThemeChromiumAndroid)

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