[Webkit-unassigned] [Bug 75553] [GTK][WK2] Initial FullScreen support

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Philippe Normand <pnormand at igalia.com>  2012-01-05 01:46:33 PST ---
(In reply to comment #4)
> (From update of attachment 121118 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121118&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:86
> > +    RefPtr<FullScreenWindowController> fullScreenWindowController;
> 
> I think this should be part of WebKitWebViewBase, not WebKitWebView, otherwise fullscreen won't work when using the C API.
> 

Ok.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:481
> > +#if ENABLE(FULLSCREEN_API)
> > +    priv->pageProxy->fullScreenManager()->setWebView(webkitWebViewBase);
> > +#endif
> 
> I think we can just ignore setWebView(). The fullscreen manager is created with a WebPageProxy that already has a method to get the view WebPageProxy::viewWidget();
> 

Hum will check this again, IIRC I just did like mac :)

> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:24
> > +#include "FullScreenWindowController.h"
> 
> I think we can avoid this object and move the implementation to WebKitWebViewBase
> 

Not sure, if we plan to implement the animation if might be better to keep this code contained in a separate class.

> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:26
> > +#include "IntRect.h"
> 
> Use <WebCore/IntRect.h>, we prefer angle-bracket includes for non wk2 headers.
> 

Oh, ok! Not so used to WK2 yet :)

> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:61
> > +    case 'f':
> > +    case 'F':
> 
> Use GDK_KEY_f and GDK_KEY_F.

Ok

> Is it possible to exit fullscreen without the keyboard?
> 

It depends! If the fullscreen element is a video you can exit fullscreen by clicking on the media-controls fullscreen button. Otherwise, no way yet.

There are 3 issues to address:

- when the window is fullscreened the UA should be notified so it can hide the widgets that are part of the window (like the top-bar of the gtk-launcher, for example)
- notify the user he can exit with the keyboard, f, F or escape
- somehow allow mouse interaction to exit fullscreen in all cases.


> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:83
> > +    gdk_window_fullscreen(gtk_widget_get_parent_window(GTK_WIDGET(m_webView.get())));
> 
> You should use topLevelWindow instead of getting the view parent window. Or just call gtk_window_fullscreen().
> 

Ok, will try that, not sure anymore why I used the GdkWindow approach :)

> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:100
> > +    gdk_window_unfullscreen(gtk_widget_get_parent_window(GTK_WIDGET(m_webView.get())));
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:111
> > +    GdkWindow* window = gtk_widget_get_parent_window(GTK_WIDGET(m_webView.get()));
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:113
> > +    gdk_window_get_frame_extents(window, &rect);
> 
> Isn't the FullScreenRect the screenRect()?
> 

Could be yes. Will double-check.

[...]

> > Source/WebKit2/UIProcess/gtk/WebFullScreenManagerProxyGtk.cpp:100
> > -    notImplemented();
> > +    if (!m_webView)
> > +        return;
> > +
> > +    WebKitWebView* webView = WEBKIT_WEB_VIEW(m_webView);
> > +    RefPtr<FullScreenWindowController> controller = webkitWebViewFullscreenWindowController(webView);
> > +    rect = controller->getFullScreenRect();
> 
> Can we use PlatformScreen::screenRect() from WebCore?

Probably yes. Will check it, thanks for the review!

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