Skip to content

Conversation

@ptziegler
Copy link
Contributor

@ptziegler ptziegler commented Dec 1, 2025

When the children of a previously expanded TreeItem are removed, the call to getExpanded() should continue to return "true". On both Linux and MacOS, this property is not persisted and therefore stored in a local variable. But on Linux, a call to getExpanded() still returns the result from a call to the GTK API.

To harmonize the behavior between the different operating systems, following changes are done:

  1. The call to getExpanded() now always returns the local variable, similar to how it's done for MacOS.

  2. The call to setExpanded() doesn't modify the tree item if it is already expanded or a leaf node.

@eclipse-platform-bot
Copy link
Contributor

eclipse-platform-bot commented Dec 1, 2025

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF
tests/org.eclipse.swt.tests/pom.xml

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From f9541c96bcbbe4b6a7a3854064f15885f4dbaadf Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Tue, 2 Dec 2025 09:50:39 +0000
Subject: [PATCH] Version bump(s) for 4.39 stream


diff --git a/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF
index 9f1d3b1108..4b571c7fe2 100644
--- a/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Eclipse SWT Tests
 Bundle-SymbolicName: org.eclipse.swt.tests
-Bundle-Version: 3.107.1000.qualifier
+Bundle-Version: 3.107.1100.qualifier
 Bundle-Vendor: Eclipse.org
 Export-Package: org.eclipse.swt.tests.junit,
  org.eclipse.swt.tests.junit.performance
diff --git a/tests/org.eclipse.swt.tests/pom.xml b/tests/org.eclipse.swt.tests/pom.xml
index 18ae2d2c43..1675fabcb9 100644
--- a/tests/org.eclipse.swt.tests/pom.xml
+++ b/tests/org.eclipse.swt.tests/pom.xml
@@ -20,7 +20,7 @@
     <relativePath>../../local-build/local-build-parent/</relativePath>
   </parent>
   <artifactId>org.eclipse.swt.tests</artifactId>
-  <version>3.107.1000-SNAPSHOT</version>
+  <version>3.107.1100-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
   <properties>
     <tycho.testArgLine></tycho.testArgLine>
-- 
2.51.2

Further information are available in Common Build Issues - Missing version increments.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

Test Results

  147 files  ±0    147 suites  ±0   20m 22s ⏱️ - 2m 9s
4 673 tests ±0  4 651 ✅ ±0  22 💤 ±0  0 ❌ ±0 
  414 runs  ±0    409 ✅ ±0   5 💤 ±0  0 ❌ ±0 

Results for commit 4f51c0b. ± Comparison against base commit 407cb9a.

♻️ This comment has been updated with latest results.

@ptziegler ptziegler force-pushed the issue2834 branch 5 times, most recently from f83432e to 84bda53 Compare December 2, 2025 10:13
@ptziegler ptziegler marked this pull request as ready for review December 4, 2025 17:48
…m#2834

When the children of a previously expanded TreeItem are removed, the
call to getExpanded() should continue to return "true". On both Linux
and MacOS, this property is not persisted and therefore stored in a
local variable. But on Linux, a call to getExpanded() still returns the
result from a call to the GTK API.

To harmonize the behavior between the different operating systems,
following changes are done:

1) The call to getExpanded() now always returns the local variable,
similar to how it's done for MacOS.

2) The call to setExpanded() doesn't modify the tree item if it is
already expanded or a leaf node.

Closes eclipse-platform#2834
@ptziegler
Copy link
Contributor Author

I've tested this with eclipse-platform/eclipse.platform.ui#3527 and now the issue also disappears on Linux. From my side this is done and any feedback is greatly appreciated.

@vogella
Copy link
Contributor

vogella commented Dec 4, 2025

Here is the feedback from Gemini CLI:

The Pull Request #2836: Make TreeItem.getExpanded consistent across platforms aims to harmonize the behavior of TreeItem
between Linux (GTK) and macOS.

Here is an analysis of the changes:

Summary
The core issue addressed is that on Linux, TreeItem.getExpanded() was directly querying the underlying GTK widget. If a
TreeItem's children were removed, GTK would treat it as a leaf (non-expandable), causing getExpanded() to return false even
if it was previously expanded. On macOS, this state is persisted in a local variable, returning true even after children
are removed. This PR adopts the macOS approach for Linux.

Key Changes

  1. TreeItem.java (GTK implementation):

    • getExpanded(): Now returns a local isExpanded field instead of calling GTK.gtk_tree_view_row_expanded. This
      ensures the state is remembered purely within SWT.
    • setExpanded(boolean):
      • Added a check GTK.gtk_tree_model_iter_n_children(...) != 0.
      • The method now does nothing if the item is a leaf (has 0 children). It neither calls GTK functions nor updates
        the isExpanded field.
      • This effectively "locks" the expansion state when an item becomes a leaf.
  2. Tests (Test_org_eclipse_swt_widgets_TreeItem.java):

    • Updated test_getExpanded to verify:
      • setExpanded(true) on a fresh leaf does not change getExpanded() (stays false).
      • Removing children from an expanded item preserves getExpanded() == true.
      • Calling setExpanded(false) on that now-leaf item does not change getExpanded() (stays true).

Assessment
The changes accurately implement the stated goal.

  • Consistency: It ensures Linux behavior matches macOS regarding state persistence.
  • Logic: The updated setExpanded logic ensures that the "virtual" expansion state isn't lost or inadvertently flipped when
    the item temporarily lacks children.
  • Verification: The added tests cover the edge cases (leaf interaction and state persistence after child removal).

Overall, the change looks correct and safe, improving cross-platform consistency for TreeItem state management.

@iloveeclipse
Copy link
Member

@ptziegler : many thanks for fix & testing. Let merge this ASAP, to get as much coverage as possible.

@iloveeclipse
Copy link
Member

Verified on my RHEL 9.6 & platform UI patch, everything works as expected.

@iloveeclipse iloveeclipse merged commit 075a512 into eclipse-platform:master Dec 5, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants