Skip to content

Conversation

@ptziegler
Copy link
Contributor

@ptziegler ptziegler commented Apr 2, 2025

For e.g. the text widget, a default pop-up menu if provided by GTK and
activated, unless a custom menu is set within SWT. To avoid both menus
being shown at the same time, the event must be consumed when showing
the SWT menu.

In GTK3, this is done by returning TRUE in the callback method, which is
no longer possible in GTK4. Instead, the event needs to be marked as
"claimed" via gtk_gesture_set_sequence_state(), to prevent other, native
listeners from processing it as well.

To do so, the gtk_gesture_press_event() and gtk_gesture_release_event()
have been adapted to now return an integer, which may be one of the
following values:

  • GTK_EVENT_SEQUENCE_NONE
  • GTK_EVENT_SEQUENCE_CLAIMED
  • GTK_EVENT_SEQUENCE_DENIED

For example, the value should be GTK_EVENT_SEQUENCE_CLAIMED after the
menu is shown.

Note: It is not possible to always claim an event as "claimed", as it
would then prevent the default value from being shown at all, even if no
SWT menu is set.

Additionally, the check for whether a menu can be shown has been
inverted ((state & MENU) != 0), to match the GTK3 logic. Because this
bit is always set, this effectively stops all SWT menus from being
shown.

@ptziegler
Copy link
Contributor Author

Left is how it currently looks like, right is how it's supposed to look like. The current menu is the default one provided by GTK and not the one that's created in Snippet 122.

image image

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025

Test Results

   408 files   -   137     408 suites   - 137   18m 16s ⏱️ - 11m 12s
 4 332 tests  -    37   4 322 ✅  -    35  10 💤  -  2  0 ❌ ±0 
12 521 runs   - 4 103  12 424 ✅  - 4 090  97 💤  - 13  0 ❌ ±0 

Results for commit b083dda. ± Comparison against base commit e218c17.

This pull request removes 37 tests.
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_emptyString
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letterA
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letters
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16LE_null
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letter
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letters
…

♻️ This comment has been updated with latest results.

@ptziegler ptziegler marked this pull request as draft April 3, 2025 04:34
@akurtakov
Copy link
Member

I tried running this PR and it crashed with

Exception in thread "main" java.lang.UnsatisfiedLinkError: 'long org.eclipse.swt.internal.gtk3.GTK3.gtk_clipboard_wait_for_contents(long, long)'
	at org.eclipse.swt.internal.gtk3.GTK3.gtk_clipboard_wait_for_contents(Native Method)
	at org.eclipse.swt.dnd.Clipboard.gtk_clipboard_wait_for_contents(Clipboard.java:701)
	at org.eclipse.swt.dnd.Clipboard.getAvailableClipboardTypes(Clipboard.java:679)
	at org.eclipse.swt.dnd.Clipboard.getAvailableTypes(Clipboard.java:576)
	at org.eclipse.swt.dnd.Clipboard.getAvailableTypes(Clipboard.java:544)
	at org.eclipse.swt.snippets.Snippet122.lambda$2(Snippet122.java:70)
	at org.eclipse.swt.events.MenuListener$2.menuShown(MenuListener.java:80)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:293)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5864)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1652)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1678)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1657)
	at org.eclipse.swt.widgets.Menu._setVisible(Menu.java:290)
	at org.eclipse.swt.widgets.Display.runPopups(Display.java:5116)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4515)
	at org.eclipse.swt.snippets.Snippet122.main(Snippet122.java:83)

Do you experience that ?

@ptziegler
Copy link
Contributor Author

ptziegler commented Apr 3, 2025

I tried running this PR and it crashed with

Exception in thread "main" java.lang.UnsatisfiedLinkError: 'long org.eclipse.swt.internal.gtk3.GTK3.gtk_clipboard_wait_for_contents(long, long)'
	at org.eclipse.swt.internal.gtk3.GTK3.gtk_clipboard_wait_for_contents(Native Method)
	at org.eclipse.swt.dnd.Clipboard.gtk_clipboard_wait_for_contents(Clipboard.java:701)
	at org.eclipse.swt.dnd.Clipboard.getAvailableClipboardTypes(Clipboard.java:679)
	at org.eclipse.swt.dnd.Clipboard.getAvailableTypes(Clipboard.java:576)
	at org.eclipse.swt.dnd.Clipboard.getAvailableTypes(Clipboard.java:544)
	at org.eclipse.swt.snippets.Snippet122.lambda$2(Snippet122.java:70)
	at org.eclipse.swt.events.MenuListener$2.menuShown(MenuListener.java:80)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:293)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5864)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1652)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1678)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1657)
	at org.eclipse.swt.widgets.Menu._setVisible(Menu.java:290)
	at org.eclipse.swt.widgets.Display.runPopups(Display.java:5116)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4515)
	at org.eclipse.swt.snippets.Snippet122.main(Snippet122.java:83)

Do you experience that ?

Yes. Locally I've adapted Clipboard.gtk_clipboard_wait_for_contents() to always return 0 when running with GTK4.

And just as a general update: I'll plan on creating a separate PR to properly implement the event cancellation, because the current approach in this PR of always consuming the event doesn't cover all cases.

In short: If no menu is set, the default menu should be used. At least that's how it currently works with GTK3 and something you can see in the snippet if you comment out the call to setMenu(...). When the event is always consumed, this no longer happens.

Conversly, if the event is never consumed, you get both the default menu and the SWT menu on top of each other...

@ptziegler ptziegler added the gtk4 GTK4 issues label Apr 3, 2025
@ptziegler ptziegler changed the title [GTK4] Fix context menu not being shown [GTK4] Manage propagation for gesture events and fix popup menus Apr 3, 2025
@ptziegler ptziegler marked this pull request as ready for review April 3, 2025 18:17
@ptziegler
Copy link
Contributor Author

@akurtakov I've tried to find another way to produce a "bubbling event", but so far without success. So in the end, I've combined everything in a single commit.

For testing, I've used Snippet 122 with and without a menu, together with #1990.

For e.g. the text widget, a default pop-up menu if provided by GTK and
activated, unless a custom menu is set within SWT. To avoid both menus
being shown at the same time, the event must be consumed when showing
the SWT menu.

In GTK3, this is done by returning TRUE in the callback method, which is
no longer possible in GTK4. Instead, the event needs to be marked as
"claimed" via gtk_gesture_set_sequence_state(), to prevent other, native
listeners from processing it as well.

To do so, the gtk_gesture_press_event() and gtk_gesture_release_event()
have been adapted to now return an integer, which may be one of the
following values:

- GTK_EVENT_SEQUENCE_NONE
- GTK_EVENT_SEQUENCE_CLAIMED
- GTK_EVENT_SEQUENCE_DENIED

For example, the value should be GTK_EVENT_SEQUENCE_CLAIMED after the
menu is shown.

Note: It is not possible to always claim an event as "claimed", as it
would then prevent the default value from being shown at all, even if no
SWT menu is set.

Additionally, the check for whether a menu can be shown has been
inverted ((state & MENU) != 0), to match the GTK3 logic. Because this
bit is always set, this effectively stops all SWT menus from being
shown.
@akurtakov
Copy link
Member

This is clear improvement to the current state thus pushing. Further improvements (if needed) should be handled separately.

@akurtakov akurtakov merged commit 1119bac into eclipse-platform:master Apr 8, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gtk4 GTK4 issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants