Summary: | [GTK] Stop accessing GdkEvent fields when possible | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ludovico de Nittis <ludovico.denittis> | ||||||||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | berto, bugs-noreply, cgarcia, commit-queue, ews-watchlist, gustavo, mcatanzaro | ||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Attachments: |
|
Description
Ludovico de Nittis
2019-06-13 07:09:12 PDT
Created attachment 372053 [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 /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. Meh apparently GTK 2 is not happy. I'm on it. 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 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
Created attachment 372110 [details]
Patch
Created attachment 372112 [details]
Patch
Created attachment 372114 [details]
Patch
Created attachment 372116 [details]
Patch
Sorry for the multiple patches. This time should be everything ok. 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. Created attachment 372124 [details]
Patch
Comment on attachment 372124 [details] Patch Clearing flags on attachment: 372124 Committed r246467: <https://trac.webkit.org/changeset/246467> All reviewed patches have been landed. Closing bug. Reverted r246467 for reason: It broke scrolling with mouse wheel Committed r246494: <https://trac.webkit.org/changeset/246494> 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" Ok, I'm able to reproduce the problem. I'll upload soon a patch for it. Created attachment 372243 [details]
Patch
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. Comment on attachment 372243 [details] Patch Clearing flags on attachment: 372243 Committed r246496: <https://trac.webkit.org/changeset/246496> All reviewed patches have been landed. Closing bug. |