[Webkit-unassigned] [Bug 76166] [WK2][GTK] FullScreen signals

Wed Feb 1 07:26:10 PST 2012

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

--- Comment #8 from Philippe Normand <pnormand at igalia.com>  2012-02-01 07:26:10 PST ---
(In reply to comment #6)
> (From update of attachment 124730 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124730&action=review
> 
> 
> You should subclass this fixture instead of just adding on to it.

Seriously? Why don't the other tests of this file subclass the fixture then?

> Why do you destroy the WebView here?

Because in the case where fullscreen is blocked the leavingFullScreen() is not called.

> The test fixture cleans up the WebView.  Please do not use C style casts. 
> 

Ok.

> You should be careful to keep the knowedge of the fixture internal to the fixture. If the fixture needs to call emitKeyStroke, then emitKeyStroke should be a static method of the fixture.

Ok.

> Why do you call g_timeout_add instead of something like g_idle_add? 200 milliseconds seems like a long time to wait.
> 

Right.

[...]

> 
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:425
> > +    test->loadHtml("<html><body>"
> > +                   "<script>"
> > +                   "var eventName = 'keypress';"
> > +                   "function thunk() {"
> > +                   "    document.removeEventListener(eventName, thunk, false);"
> > +                   "    document.documentElement.webkitRequestFullScreen();"
> > +                   "}"
> > +                   "document.addEventListener(eventName, thunk, false);"
> > +                   "</script></body></html>", 0);
> > +
> > +    GtkWidget* window;
> > +    window = gtk_window_new(GTK_WINDOW_POPUP);
> > +    gtk_container_add(GTK_CONTAINER(window), GTK_WIDGET(test->m_webView));
> > +
> > +    gtk_widget_show_all(window);
> > +    g_timeout_add(100, (GSourceFunc) emitKeyStroke, test);
> > +
> > +    test->waitUntilMainLoopFinishes();
> 
> It seems like this is very similar to the code above. Can you refactor it so that it's shared?
> 

Yes. I will add a Test::addWithData(const char* suiteName, const char* testName, void (*testFunc)(ClassName*, const void*), gconstpointer data) then.

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