[webkit-reviews] review granted: [Bug 75553] [GTK][WK2] Initial FullScreen support : [Attachment 134767] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 75553: [GTK][WK2] Initial FullScreen support
https://bugs.webkit.org/show_bug.cgi?id=75553

Attachment 134767: Patch
https://bugs.webkit.org/attachment.cgi?id=134767&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134767&action=review


>> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:81
>> +	bool fullScreenEnabled;
> 
> It seems this is only used inside #if ENABLE(FULLSCREEN_API) #endif blocks,
so maybe we should add #if here?

For the naming of this perhaps "inFullScreenMode", "fullScreenModeActive", or
"fullScreenActive" would make it clearer.

As an asides, it's a bit odd that in WebKit2 it's referred to as "FullScreen"
(i.e. "full screen"), but the guard is "FULLSCREEN" (i.e. "fullscreen").

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:293
> +	   switch (event->keyval) {
> +	   case GDK_KEY_Escape:
> +	   case GDK_KEY_f:
> +	   case GDK_KEY_F:
> +	       webkitWebViewBaseExitFullScreen(webViewBase);

I guess this will be an issue if a game wants to go fullscreen and use the 'f'
key?