[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

--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Adrian Perez from comment #4)
> Comment on attachment 400116 [details]
> 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.)

It makes sense, but we already use WallTime for events, so if we want to change that it must happen in a separate patch to fix all other places where we already use WallTime.

> > 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.

Sure!

> > 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)”.

You could have read the ChangeLog :-)

> Could you please add a comment here mentioning that?

Sure!

> (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.

It didn't work.

-- 
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/20200524/e01edab6/attachment.htm>