From 12b780e5c5627560612fe7410d47b0e58eafd1b8 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Mon, 1 Jun 2026 10:34:54 +0200 Subject: [PATCH] Make CTabFolder.setItemOrder package-private again, keep moveItem Following review feedback, setItemOrder(int[]) should not be exposed as public API. Demote it back to a package-private helper and drop its @since tag, but keep the public moveItem(int, int) method as the supported single-tab reorder primitive. The priority[]/firstIndex remap fix is retained since moveItem relies on it for correct chevron and scroll state. Removes the now-uncompilable setItemOrder unit tests. --- .../org/eclipse/swt/custom/CTabFolder.java | 31 ++-- ...est_org_eclipse_swt_custom_CTabFolder.java | 134 ------------------ 2 files changed, 8 insertions(+), 157 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java b/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java index 503d1bd634..45f081f12e 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java @@ -2843,22 +2843,15 @@ boolean setItemLocation(GC gc) { return changed; } /** - * Sets the order that the items in the receiver should be displayed in to the - * given argument. + * Reorder the items of the receiver. *

- * The argument is described in terms of the zero-relative ordering of the - * receiver's current items: indices[i] is the index of the item - * that should be moved to position i. In other words, the item - * currently at index indices[i] becomes the new item at index - * i. The argument must contain every index of the receiver's items - * exactly once. - *

- * The currently selected item remains selected after the reorder, and the set - * of visible tabs is preserved as far as the new ordering allows. + * indices[i] is the index of the item that should be moved to + * position i. The argument must contain every index of the + * receiver's items exactly once. The currently selected item remains selected + * after the reorder. *

* - * @param indices the new display order of the receiver's items, expressed as a - * permutation of their current indices + * @param indices an array containing the new indices for all items * * @exception IllegalArgumentException - * - * @see #moveItem(int, int) - * - * @since 3.135 */ -public void setItemOrder (int[] indices) { +/*public*/ void setItemOrder (int[] indices) { checkWidget(); if (indices == null) SWT.error (SWT.ERROR_NULL_ARGUMENT); if (indices.length != items.length) SWT.error (SWT.ERROR_INVALID_ARGUMENT); @@ -2907,9 +2896,7 @@ public void setItemOrder (int[] indices) { * Moves the item at the from index to the to index. * The items between the two indices shift by one position to fill the gap; all * other items keep their relative order. This is a convenience for the common - * single-item reorder gesture (such as dragging a tab) and is equivalent to - * building the corresponding permutation and passing it to - * {@link #setItemOrder(int[])}. + * single-item reorder gesture, such as dragging a tab. *

* If from and to are equal this method has no effect. * The currently selected item remains selected after the move. @@ -2928,8 +2915,6 @@ public void setItemOrder (int[] indices) { *

  • ERROR_THREAD_INVALID_ACCESS - if not called from the thread that created the receiver
    • * * - * @see #setItemOrder(int[]) - * * @since 3.135 */ public void moveItem (int from, int to) { diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java index ad95acbefe..ac61c9e298 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java @@ -1050,104 +1050,6 @@ public void test_dirtyIndicator_closesWhenCloseEnabled() { } } -@Test -public void test_setItemOrder_reordersItems() { - createTabFolder(null, 4); - CTabItem[] original = ctabFolder.getItems(); - - // indices[i] is the index of the item that should move to position i - ctabFolder.setItemOrder(new int[] { 2, 0, 3, 1 }); - - CTabItem[] reordered = ctabFolder.getItems(); - assertEquals(original[2], reordered[0]); - assertEquals(original[0], reordered[1]); - assertEquals(original[3], reordered[2]); - assertEquals(original[1], reordered[3]); -} - -@Test -public void test_setItemOrder_keepsSelection() { - createTabFolder(null, 4); - CTabItem selected = ctabFolder.getItem(1); - ctabFolder.setSelection(selected); - assertEquals(selected, ctabFolder.getSelection()); - - ctabFolder.setItemOrder(new int[] { 3, 1, 0, 2 }); - - // the same item must still be selected, with its index updated - assertEquals(selected, ctabFolder.getSelection()); - assertEquals(1, ctabFolder.getSelectionIndex()); - assertEquals(selected, ctabFolder.getItem(1)); -} - -@Test -public void test_setItemOrder_identityIsNoOp() { - createTabFolder(null, 4); - CTabItem[] original = ctabFolder.getItems(); - ctabFolder.setSelection(ctabFolder.getItem(2)); - - ctabFolder.setItemOrder(new int[] { 0, 1, 2, 3 }); - - assertArrayEquals(original, ctabFolder.getItems()); - assertEquals(2, ctabFolder.getSelectionIndex()); -} - -@Test -public void test_setItemOrder_chevronStillWorks() { - createTabFolder(null, 8); - shell.setSize(800, 200); - processEvents(); - // shrink so that not all tabs fit and a chevron is required - showChevron(); - processEvents(); - - CTabItem selected = ctabFolder.getItem(0); - ctabFolder.setSelection(selected); - processEvents(); - - // reverse the order - int n = ctabFolder.getItemCount(); - int[] order = new int[n]; - for (int i = 0; i < n; i++) { - order[i] = n - 1 - i; - } - ctabFolder.setItemOrder(order); - processEvents(); - - // the chevron must still be present and the internal book-keeping consistent - assertNotNull(getChevron(ctabFolder), "Chevron should still be shown after reordering"); - assertEquals(selected, ctabFolder.getSelection()); - - int[] priority = reflection_getIntArray(ctabFolder, "priority"); - assertTrue(isPermutation(priority, n), "priority[] must remain a permutation of item indices after reorder"); - int firstIndex = reflection_getInt(ctabFolder, "firstIndex"); - assertTrue(firstIndex >= -1 && firstIndex < n, "firstIndex must reference a valid item after reorder"); - - // no tabs / toolbar items overlap after the reorder - checkElementOverlap(ctabFolder); -} - -@Test -public void test_setItemOrder_errorCases() { - createTabFolder(null, 3); - - assertThrows(IllegalArgumentException.class, () -> ctabFolder.setItemOrder(null), - "null argument must be rejected"); - assertThrows(IllegalArgumentException.class, () -> ctabFolder.setItemOrder(new int[] { 0, 1 }), - "wrong-length array must be rejected"); - assertThrows(IllegalArgumentException.class, () -> ctabFolder.setItemOrder(new int[] { 0, 1, 1 }), - "duplicate index must be rejected"); - assertThrows(IllegalArgumentException.class, () -> ctabFolder.setItemOrder(new int[] { 0, 1, 3 }), - "out-of-range index must be rejected"); - assertThrows(IllegalArgumentException.class, () -> ctabFolder.setItemOrder(new int[] { -1, 1, 2 }), - "negative index must be rejected"); - - // a rejected call must leave the existing order untouched - CTabItem[] original = ctabFolder.getItems(); - assertThrows(IllegalArgumentException.class, () -> ctabFolder.setItemOrder(new int[] { 0, 1, 1 })); - assertArrayEquals(original, ctabFolder.getItems()); -} - @Test public void test_moveItem_forward() { createTabFolder(null, 5); @@ -1215,40 +1117,4 @@ public void test_moveItem_errorCases() { "out-of-range to index must be rejected"); } -private static int[] reflection_getIntArray(CTabFolder tabFolder, String fieldName) { - try { - Field field = CTabFolder.class.getDeclaredField(fieldName); - field.setAccessible(true); - return (int[]) field.get(tabFolder); - } catch (Exception e) { - fail("Failed to access " + fieldName + " via reflection: " + e.getMessage()); - return null; - } -} - -private static int reflection_getInt(CTabFolder tabFolder, String fieldName) { - try { - Field field = CTabFolder.class.getDeclaredField(fieldName); - field.setAccessible(true); - return field.getInt(tabFolder); - } catch (Exception e) { - fail("Failed to access " + fieldName + " via reflection: " + e.getMessage()); - return -1; - } -} - -private static boolean isPermutation(int[] values, int size) { - if (values.length != size) { - return false; - } - boolean[] seen = new boolean[size]; - for (int value : values) { - if (value < 0 || value >= size || seen[value]) { - return false; - } - seen[value] = true; - } - return true; -} - }