Skip to content

Conversation

@raghucssit
Copy link
Contributor

@raghucssit raghucssit commented Sep 18, 2025

We cannot set/determine the location of the shell using Window Handle on
GTK4. So fix the test not to test the set and get location on GTK4.

see #2498

@raghucssit
Copy link
Contributor Author

@iloveeclipse please add GTK4 label.

@iloveeclipse iloveeclipse added the gtk4 GTK4 issues label Sep 18, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 18, 2025

Test Results

   88 files   - 30     88 suites   - 30   6m 26s ⏱️ - 2m 21s
4 395 tests  - 37  4 375 ✅  - 35  15 💤  - 2  5 ❌ ±0 
  237 runs   - 61    229 ✅  - 60   3 💤  - 1  5 ❌ ±0 

For more details on these failures, see this check.

Results for commit b938577. ± Comparison against base commit 79e69bb.

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

♻️ This comment has been updated with latest results.

@akurtakov
Copy link
Member

So if one sets location via API and user moves the shell returned location will be not where it was moved but rather what was initially set via the API?!? This looks like totally broken logic.

@iloveeclipse
Copy link
Member

iloveeclipse commented Sep 18, 2025

If I remember right, GTK4 provides only relative coordinates, so can we record the diff to absolute coordinates set by user, and apply the diff on next get to the value reported by GTK4?

@laeubi
Copy link
Contributor

laeubi commented Sep 18, 2025

So if one sets location via API and user moves the shell returned location will be not where it was moved but rather what was initially set via the API?!? This looks like totally broken logic.

One might want to attach a move listener to fix that, but one should also note that not all shells are movable and it is better than having no location reporting at all, so not optimal but a first step maybe...

@akurtakov
Copy link
Member

If I remember right, GTK4 provides only relative coordinates, so can we record the diff to absolute coordinates set by user, and apply the diff on next get to the value reported by GTK4?

I should start with the fact that absolute coordinates set by user will have zero effect when running on gtk3+Wayland(https://docs.gtk.org/gdk3/method.Window.move.html) or gtk4 (

). With the above said such a thing have to be limited to gtk3+x11 where things already behave like that even without such a hack.

@akurtakov
Copy link
Member

https://discourse.gnome.org/t/how-to-center-gtkwindows-in-gtk4/3112/11 and the whole thread is worth reading on the topic from one of the main gtk developers.

@akurtakov
Copy link
Member

@raghucssit
Copy link
Contributor Author

After some tries and readings.. I conclude that it is not possible to know Location of the Top Level Window.
Shell.gtk_configure_event(long, long) is triggered only on GTK3 but not for GTK4.
So there is a listener already to receive the configure event but it's never called by GTK4 because it has removed gtk_window_get_position().
So we can only fix the test and update API doc.

@raghucssit raghucssit force-pushed the gtk4_shell_location_fix branch 2 times, most recently from 40d0d06 to f9d6ffc Compare September 19, 2025 11:48
@Test
public void test_getLocation() {
//Setting location for Windows is not supported in GTK4
if (GTK.GTK4) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you run the test on Windows/Mac - class GTK will not be available and ClassNotFoundException will be thrown.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, then how to disable these tests for GTK4 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any test parameter or maven configuration ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's best to use the osgi.ws system property equals to gtk, combined with org.eclipse.swt.internal.gtk.version system property startsWith("4"). That way test for gtk 3.x will still execute and only gtk 4.x will be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's best to use the osgi.ws system property equals to gtk, combined with org.eclipse.swt.internal.gtk.version system property startsWith("4"). That way test for gtk 3.x will still execute and only gtk 4.x will be disabled.

Thank you.. It works well.. I have fixed it now..

We cannot set/determine the location of the shell using Window Handle on
GTK4. So fix the test not to test the set and get location on GTK4.

see eclipse-platform#2498
@raghucssit raghucssit force-pushed the gtk4_shell_location_fix branch from f9d6ffc to b938577 Compare September 19, 2025 12:38
@raghucssit raghucssit changed the title GTK4: Fix GTK4 shell location. GTK4: Fix GTK4 shell location test. Sep 19, 2025
@iloveeclipse
Copy link
Member

@raghucssit , @akurtakov : I think we should update Javadoc on affected SWT methods (get/set) to add a note about this GTK4 limitation?

Beside this: wdyt about adding some logging callback from IDE to SWT, so that it could be used to report errors in the IDE log if some unexpected (not critical) SWT error appears? So far SWT can only throw exceptions or print to sysout, but I would prefer also to log missing functionality like here to IDE error log without breaking callers. WDYT?

@raghucssit
Copy link
Contributor Author

I think we should update Javadoc on affected SWT methods (get/set) to add a note about this GTK4 limitation?

I will do this with this PR.

@raghucssit , @akurtakov : I think we should update Javadoc on affected SWT methods (get/set) to add a note about this GTK4 limitation?

Beside this: wdyt about adding some logging callback from IDE to SWT, so that it could be used to report errors in the IDE log if some unexpected (not critical) SWT error appears? So far SWT can only throw exceptions or print to sysout, but I would prefer also to log missing functionality like here to IDE error log without breaking callers. WDYT?

I don't know how to do this.. May be we can do it with different PR because improving logging mechanism altogether seems some enhancement.

@iloveeclipse
Copy link
Member

I don't know how to do this.. May be we can do it with different PR because improving logging mechanism altogether seems some enhancement.

Sure, I was interesting to hear your opinion.

@raghucssit
Copy link
Contributor Author

I will do this with this PR.

Already API doc has the needed warning for wayland windowing system. I think it's enough. Can we say it as may be GTK4 limitation ?

setLocation()
 * <b>Warning:</b> When executing this operation on a shell, it may not have the
 * intended effect on some platforms. For example, executing this operation on a
 * shell when the environment uses the Wayland protocol, nothing will happen.
getLocation()
 * <b>Warning:</b> When executing this operation on a shell, it may not yield a
 * value with the expected meaning on some platforms. For example, executing
 * this operation on a shell when the environment uses the Wayland protocol, the
 * result is <b>not</b> a coordinate relative to the display. It will not change
 * when moving the shell.
 *

@raghucssit
Copy link
Contributor Author

Shell test has only 1 fail now. So this PR has effect.. Rest everything is ok. Good to merge.

Z [ERROR] Tests run: 140, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 10.40 s <<< FAILURE! -- in org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Shell
2025-09-19T12:44:51.0459068Z [ERROR] org.eclipse.swt.tests.junit.AllTests.test_Issue450_NoShellActivateOnSetFocus -- Time elapsed: 4.969 s <<< FAILURE!

@akurtakov
Copy link
Member

Technically it's Gtk 4.x design decision in order to fix security concern so it's not a limitation but rather a deliberate decision.

@akurtakov akurtakov merged commit a479377 into eclipse-platform:master Sep 19, 2025
14 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.

4 participants