[Webkit-unassigned] [Bug 212298] [WTR] EventSender: stop using GdkEvent API in preparation for GTK4

[bugzilla-daemon at webkit.org]

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

Adrian Perez <aperez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aperez at igalia.com
 Attachment #400116|review?                     |review-
              Flags|                            |

--- Comment #4 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 400116
  --> https://bugs.webkit.org/attachment.cgi?id=400116
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400116&action=review

> Source/WebKit/Shared/gtk/NativeWebKeyboardEventGtk.cpp:43
> +    : WebKeyboardEvent(type, text, key, code, keyIdentifier, windowsVirtualKeyCode, nativeVirtualKeyCode, false, WTF::nullopt, WTF::nullopt, WTFMove(commands), isKeypad, modifiers, WallTime::now())

Shouldn't events rather use “MonotonicTime” instead? Using “WallTime” can
result in odd situations where an event created later in time has an older
timestamp if the system clock gets adjusted in between, which would break
their sequencing.

(System clock adjustments can happen at any time if there is a NTP client
running: if it detects that it cannot correct drift with small incremental
adjustments it may decide to to a bigger time bump and go back in time.
Another case is the user changing timezones e.g. while traveling.)

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2356
> +    gdk_window_get_root_coords(gtk_widget_get_window(GTK_WIDGET(webViewBase)), x, y, &xRoot, &yRoot);

See next comment below in line 2469.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2469
> +    gdk_window_get_root_coords(gtk_widget_get_window(GTK_WIDGET(webViewBase)), x, y, &xRoot, &yRoot);

This same #if is repeated. I would put a “void gtkGetWidgetRootCoords()”
helper function in “GtkUtilities.h” with versions for both GTK3 and GTK4,
then use it here and also above in line 2356.

> Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.cpp:103
> +        g_object_ref(window);

Ah, it took me a while (and a detour through GTK code) to figure out
that “gdk_event_destroy()” results in calling “g_object_unref(window)”.
Could you please add a comment here mentioning that?

(Nice catch, by the way!)

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:124
> +        if (WKStringIsEqualToUTF8CString(keyRef, "rightArrow"))

Wow, out of curiosity I did a “git blame” here and it turns out that
this has gone unnoticed since 2012, and we have the same typo in
“EventSenderProxyWPE.cpp” because parts of the code were very likely
copied ��️

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:232
> +    return (WebKitWebViewBase*)view;

I think a “reinterpret_cast” should work as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200523/eb2a7058/attachment.htm>