[Webkit-unassigned] [Bug 86129] [BlackBerry] Enable the Fullscreen API

[bugzilla-daemon at webkit.org]

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


Antonio Gomes <tonikitoo at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #141483|                            |commit-queue-
               Flag|                            |




--- Comment #17 from Antonio Gomes <tonikitoo at webkit.org>  2012-05-15 07:13:51 PST ---
(From update of attachment 141483)
View in context: https://bugs.webkit.org/attachment.cgi?id=141483&action=review

We sure can review it. I have a question, a suggestion and a nitpick :)

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:127
>      if (isFullscreen()) {
> -        HTMLMediaElement* element = static_cast<HTMLMediaElement*>(m_webCorePlayer->mediaPlayerClient());
> -        element->exitFullscreen();
> +        if (HTMLMediaElement* element = static_cast<HTMLMediaElement*>(m_webCorePlayer->mediaPlayerClient()))
> +            element->exitFullscreen();

I still think it would have to be done on a separate patch. Jer?

> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.cpp:724
> +#if ENABLE(FULLSCREEN_API)
> +bool ChromeClientBlackBerry::supportsFullScreenForElement(const WebCore::Element* element, bool withKeyboard)
> +{
> +    return !withKeyboard;
> +}

I reviewed a patch for the EFL port implementation of FULLSCREEN and they are simply ignoring this 'withKeyboard' parameter, and making use of 'element' for everything.

could you explain me the difference, Max?

> Source/WebKit/blackberry/WebCoreSupport/ChromeClientBlackBerry.h:130
> +    virtual bool supportsFullScreenForElement(const Element*, bool);

we could name this bool parameter, I think.

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