[Webkit-unassigned] [Bug 97202] [GTK] Implement ViewState methods in PageClientImpl in WebKit2

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-09-20 23:49:01 PST ---
(In reply to comment #4)
> (From update of attachment 164898 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164898&action=review
> 
> This looks good to me, though changes to the test harness suggest that this will allow us to unskip tests or change results somehow.

The changes in the tests is because this patch broke several tests. Current code always returns true for isFocused, isInwindow, etc. some tests move the focus to the view and then check that the view is focused calling isFocused() and of course it was always true. Others check isInActiveWindow() with the same situation, etc. We have PlatformWebView::focus() unimplemented in this moment because our view is always focused from the API POV. With the patch all view state flags are correctly updated and we need to implement PlatformWebView::focus() to focus the view programatically. This fixed all the tests that were failing because isFocused or isInActiveWindow returned false. I'll try again with the skipped to tests to check if any of them pass now.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:100
> > +    gulong toplevelResizeGripVisibilityID;
> > +    gulong toplevelFocusInEventID;
> > +    gulong toplevelFocusOutEventID;
> 
> Maybe just unsigned long here?

g_signal_connect returns a gulong.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:120
> > +static void webkitWebViewBaseNotifyResizerSizeForWindow(WebKitWebViewBase* webViewBase)
> 
> I guess you can remove the "ForWindow" from the name since you are no longer passing it.

Ok.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:254
> > +    if (widgetIsOnscreenToplevelWindow(toplevel))
> > +        webkitWebViewBaseSetToplevelOnScreenWindow(webView, GTK_WINDOW(toplevel));
> 
> Are there situations where a window can be realized already inside a toplevel window, without set_parent being called?

A widget must be inside a toplevel to be realized, so parent_set has always been called when the widget is realized.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:743
> > +    if (widgetIsOnscreenToplevelWindow(toplevel))
> > +        webkitWebViewBaseSetToplevelOnScreenWindow(webViewBase, GTK_WINDOW(toplevel));
> > +    else
> > +        webkitWebViewBaseSetToplevelOnScreenWindow(webViewBase, 0);
> 
> If you like, this could just be:
> 
> webkitWebViewBaseSetToplevelOnScreenWindow(webViewBase, widgetIsOnscreenToplevelWindow(toplevel) ? GTK_WINDOW(toplevel) :  0);

I thought about it, but I thought the explicit if was clearer in this case.

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