Add internal API to WebKitWebViewBase to synthesize events instead, because with GTK4 it's not possible to create events. In the case of layout tests, the web view is always the target of the events, so we don't really need to send the events to GTK to process them.
Created attachment 400115 [details] Patch There's only one regression, pointer-lock/mouse-event-delivery.html I'll rework pointer lock ot ot use GdkEvents in a follow up patch.
Created attachment 400116 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
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.) > 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.
(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.
(In reply to Carlos Garcia Campos from comment #5) > (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. Ok, it was because of the const, it just needs an additional const_cast.
Created attachment 400152 [details] Patch
Created attachment 400157 [details] Patch
Committed r262184: <https://trac.webkit.org/changeset/262184>
<rdar://problem/63664943>