[Webkit-unassigned] [Bug 69410] [WK2] [GTK] Implement KeyDown function for WebKit2 EventSender.

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #111732|review?                     |review-
               Flag|                            |




--- Comment #7 from Martin Robinson <mrobinson at webkit.org>  2011-10-20 23:08:31 PST ---
(From update of attachment 111732)
View in context: https://bugs.webkit.org/attachment.cgi?id=111732&action=review

> Tools/WebKitTestRunner/GNUmakefile.am:15
>  	Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp \
> +	Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp \
>  	Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp \

We try to keep the source lists in alphabetical order. That means that EventSenderProxyGtk.cpp would be before PlatformWebViewGtk.cpp.

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:83
> +        if (WKStringIsEqualToUTF8CString(keyRef, "leftArrow"))
> +            gdkKeySym = GDK_KEY_KP_Left;
> +        else if (WKStringIsEqualToUTF8CString(keyRef, "rightArror"))
> +            gdkKeySym = GDK_KEY_KP_Right;
> +        else if (WKStringIsEqualToUTF8CString(keyRef, "upArrow"))
> +            gdkKeySym = GDK_KEY_KP_Up;

Typically we use early returns to do this sort of thing. Do you mind doing that and modifying the "else if" statements to be simply if?

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:96
> +            gdkKeySym = GDK_KEY_KP_Insert;
> +        else if (WKStringIsEqualToUTF8CString(keyRef, "delete"))

You'll need a final else here to return GDK_KEY_VoidSymbol. You might want to ASSERT_NOT_REACHED.

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:101
> +    } else {
> +        if (WKStringIsEqualToUTF8CString(keyRef, "leftArrow"))
> +            gdkKeySym = GDK_KEY_Left;
> +        else if (WKStringIsEqualToUTF8CString(keyRef, "rightArror"))

After which, you can move this out of the else block.

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:153
> +            size_t stringSize = WKStringGetLength(keyRef);
> +            char* buffer = new char[stringSize];
> +            WKStringGetUTF8CString(keyRef, buffer, stringSize);
> +            int charCode = buffer[0];
> +
> +            if (charCode == '\n' || charCode == '\r')

And this will also be out of the else block as well.

> Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp:127
> +bool PlatformWebView::dispatchEvent(GdkEvent* event)
> +{
> +    if (event->type == GDK_KEY_PRESS || event->type == GDK_KEY_RELEASE) {
> +        event->key.window = gtk_widget_get_window(GTK_WIDGET(m_view));
> +        g_object_ref(event->key.window);

Why can't this be in the EventSender?

> Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp:130
> +#ifndef GTK_API_VERSION_2
> +        gdk_event_set_device(event, getDefaultGDKPointerDevice(event->key.window));
> +#endif

There is no GTK+ 2 support in Webkit2, so you can remove this #ifdef.

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