Bug 198829 – [GTK] Stop accessing GdkEvent fields when possible

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Ludovico de Nittis]

Direct access to GdkEvent structs is no longer possible in GTK 4.
While it is not (yet?) deprecated in GTK 3, when possible I think that we should already move away from accessing directly Gdk event fields when possible.

For the majority of these fields, getters are already available in GTK 3.

[Ludovico de Nittis]

Created attachment 372053 [details]
Patch

[EWS Watchlist]

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

[Michael Catanzaro]

/home/ews/gtk-wk2-ews/WebKit/Source/WebCore/platform/gtk/PlatformWheelEventGtk.cpp: In constructor ‘WebCore::PlatformWheelEvent::PlatformWheelEvent(GdkEventScroll*)’:
/home/ews/gtk-wk2-ews/WebKit/Source/WebCore/platform/gtk/PlatformWheelEventGtk.cpp:65:5: error: ‘gdk_event_get_scroll_direction’ was not declared in this scope
     gdk_event_get_scroll_direction(reinterpret_cast<GdkEvent*>(event), &direction);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ews/gtk-wk2-ews/WebKit/Source/WebCore/platform/gtk/PlatformWheelEventGtk.cpp:65:5: note: suggested alternative: ‘gdk_event_get_screen’
     gdk_event_get_scroll_direction(reinterpret_cast<GdkEvent*>(event), &direction);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     gdk_event_get_screen
ninja: build stopped: subcommand failed.

[Ludovico de Nittis]

Meh apparently GTK 2 is not happy.
I'm on it.

[EWS Watchlist]

Comment on attachment 372053 [details]
Patch

Attachment 372053 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12465307

New failing tests:
webgl/2.0.0/conformance/context/context-release-upon-reload.html

[EWS Watchlist]

Created attachment 372059 [details]
Archive of layout-test-results from ews116 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Ludovico de Nittis]

Created attachment 372110 [details]
Patch

[Ludovico de Nittis]

Created attachment 372112 [details]
Patch

[Ludovico de Nittis]

Created attachment 372114 [details]
Patch

[Ludovico de Nittis]

Created attachment 372116 [details]
Patch

[Ludovico de Nittis]

Sorry for the multiple patches. This time should be everything ok.

[Michael Catanzaro]

Comment on attachment 372116 [details]
Patch

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

Thanks.

> Source/WebCore/platform/gtk/PlatformMouseEventGtk.cpp:58
> -    m_position = IntPoint((int)event->x, (int)event->y);
> -    m_globalPosition = IntPoint((int)event->x_root, (int)event->y_root);
> +    m_position = IntPoint((int)x, (int)y);
> +    m_globalPosition = IntPoint((int)rootX, (int)rootY);

Preexisting problem, but since you're modifying these lines, change them to static_cast because C-style casts aren't allowed in WebKit.

[Ludovico de Nittis]

Created attachment 372124 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 372124 [details]
Patch

Clearing flags on attachment: 372124

Committed r246467: <https://trac.webkit.org/changeset/246467>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

Reverted r246467 for reason:

It broke scrolling with mouse wheel

Committed r246494: <https://trac.webkit.org/changeset/246494>

[Ludovico de Nittis]

Comment on attachment 372124 [details]
Patch

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

> Source/WebKit/Shared/gtk/WebEventFactory.cpp:-210
> -    switch (event->scroll.direction) {

Carlos was the scroll broken only with GTK2 enabled? Because I found that I mistyped the direction here. It should be "direction = event->scroll.direction" instead of "direction = event->direction"

[Ludovico de Nittis]

Ok, I'm able to reproduce the problem. I'll upload soon a patch for it.

[Ludovico de Nittis]

Created attachment 372243 [details]
Patch

[Ludovico de Nittis]

The problem was created by "gdk_event_get_scroll_direction" that returns false if the direction is GDK_SCROLL_SMOOTH, so a different behavior compared to "event->direction".

Now hopefully it should be fine.

[WebKit Commit Bot]

Comment on attachment 372243 [details]
Patch

Clearing flags on attachment: 372243

Committed r246496: <https://trac.webkit.org/changeset/246496>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.