[Webkit-unassigned] [Bug 212298] [GTK][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 #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(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.

-- 
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/380eb04f/attachment-0001.htm>