Skip to content

[Gtk3] fix for SWTException on Table.remove(...) with focused row #1606 #1608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Gtk3] fix for SWTException on Table.remove(...) with focused row #1606 #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1124,11 +1124,11 @@ void destroyItem (TableItem item) {
}
if (index == itemCount) return;
long selection = GTK.gtk_tree_view_get_selection (handle);
System.arraycopy (items, index + 1, items, index, --itemCount - index);
items [itemCount] = null;
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
GTK.gtk_list_store_remove (modelHandle, item.handle);
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
System.arraycopy (items, index + 1, items, index, --itemCount - index);
items [itemCount] = null;
if (itemCount == 0) resetCustomDraw ();
}

Expand Down Expand Up @@ -2664,11 +2664,11 @@ public void remove (int index) {
}
if (!disposed) {
long selection = GTK.gtk_tree_view_get_selection (handle);
System.arraycopy (items, index + 1, items, index, --itemCount - index);
items [itemCount] = null;
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
GTK.gtk_list_store_remove (modelHandle, iter);
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
System.arraycopy (items, index + 1, items, index, --itemCount - index);
items [itemCount] = null;
}
OS.g_free (iter);
}
Expand Down Expand Up @@ -2703,20 +2703,17 @@ public void remove (int start, int end) {
long selection = GTK.gtk_tree_view_get_selection (handle);
long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ());
if (iter == 0) error (SWT.ERROR_NO_HANDLES);
int index = -1;
for (index = start; index <= end; index++) {
if (index == start) GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index);
TableItem item = items [index];
GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, start);
for (int index = start; index <= end; index++) {
TableItem item = items [start];
if (item != null && !item.isDisposed ()) item.release (false);
System.arraycopy (items, start + 1, items, start, --itemCount - start);
items [itemCount] = null;
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
GTK.gtk_list_store_remove (modelHandle, iter);
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
}
OS.g_free (iter);
index = end + 1;
System.arraycopy (items, index, items, start, itemCount - index);
for (int i=itemCount-(index-start); i<itemCount; i++) items [i] = null;
itemCount = itemCount - (index - start);
}

/**
Expand Down Expand Up @@ -2764,11 +2761,11 @@ public void remove (int [] indices) {
GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index);
}
if (!disposed) {
System.arraycopy (items, index + 1, items, index, --itemCount - index);
items [itemCount] = null;
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
GTK.gtk_list_store_remove (modelHandle, iter);
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
System.arraycopy (items, index + 1, items, index, --itemCount - index);
items [itemCount] = null;
}
last = index;
}
Expand Down
93 changes: 93 additions & 0 deletions ...t.tests.gtk/JUnit Tests/org/eclipse/swt/tests/gtk/Test_Gtk3_Table_Remove_Focused_Row.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package org.eclipse.swt.tests.gtk;

import org.eclipse.swt.SWT;
import org.eclipse.swt.internal.gtk.GTK;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableItem;
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

public class Test_Gtk3_Table_Remove_Focused_Row {

private Shell shell;
private Table table;

@BeforeClass
public static void setUpClass() {
var isGtk3 = false;
if ("gtk".equals(SWT.getPlatform())) {
@SuppressWarnings("restriction")
var gtkMajorVersion = GTK.GTK_VERSION >>> 16;
isGtk3 = (gtkMajorVersion == 3);
}
Assume.assumeTrue("Test is only for Gtk3.", isGtk3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this test fail on other platforms? If not, why should not it run there?

Copy link
Author

@om-11-2024 om-11-2024 Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The platform (as returned by org.eclipse.swt.SWT.getPlatform() ) is "gtk".
All the test in tests/gtk/... are executed only on "gtk" platform.

This test is in tests/gtk/... because it tests a bug in the swt implementation for gtk.

For now swt uses gtk3 + switching to gtk4 is in progress.
In gtk4 GtkTreeView is deprecated, which means that the code for Table for gtk4 will be rewritten.
That's why this test and this fix are only for gtk3.

Copy link
Author

Choose a reason for hiding this comment

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

Additionally.
In GTK3 this bug involves GtkCellAccessible from the so called Accessibility toolkit (ATK) used by GTK3.
In GTK4 accessibility implemented differently and ATK isn't used anymore.
As a result, even if SWT would continue to use deprecated GtkTreeView in GTK4, this bug would not happen there.

}

@Before
public void setUp() {
shell = new Shell();
shell.setMinimumSize(400, 400);
shell.setLayout(new FillLayout());
}

@After
public void tearDown() {
if (table != null) {
table.dispose();
}
if (shell != null) {
shell.dispose();
}
}

@Test
public void test_remove_focused_row_remove_one() {
test_remove_focused_row_remove_method_arg(() -> table.remove(0));
}

@Test
public void test_remove_focused_row_remove_array() {
test_remove_focused_row_remove_method_arg(() -> table.remove(new int[] { 0 }));
}

@Test
public void test_remove_focused_row_remove_interval() {
test_remove_focused_row_remove_method_arg(() -> table.remove(0, 0));
}

private void test_remove_focused_row_remove_method_arg(Runnable removeRow0) {
table = new Table(shell, SWT.VIRTUAL);
table.setItemCount(2);
table.addListener(SWT.SetData, event -> {
var item = (TableItem) event.item;
item.setText("Item #" + System.identityHashCode(item) + " " + item.toString());
});

shell.pack();
shell.open();

processUiEvents();

// set focus on row[0]
table.setFocus();

processUiEvents();

table.clear(0);
removeRow0.run();

processUiEvents();
}

private void processUiEvents() {
while (table.getDisplay().readAndDispatch()) {
// continue to next event
}
}

}
Loading