Skip to content

Commit 66d7b0c

Browse files
committed
8371657: [macosx] Programmatically selecting/deselecting List item triggers an ItemEvent
Reviewed-by: aivanov, azvegint, dnguyen, tr
1 parent 431dcf8 commit 66d7b0c

File tree

3 files changed

+215
-13
lines changed

3 files changed

+215
-13
lines changed

src/java.desktop/macosx/classes/sun/lwawt/LWListPeer.java

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -86,8 +86,11 @@ void initializeImpl() {
8686
final int[] selectedIndices = getTarget().getSelectedIndexes();
8787
synchronized (getDelegateLock()) {
8888
getDelegate().setSkipStateChangedEvent(true);
89-
getDelegate().getView().setSelectedIndices(selectedIndices);
90-
getDelegate().setSkipStateChangedEvent(false);
89+
try {
90+
getDelegate().getView().setSelectedIndices(selectedIndices);
91+
} finally {
92+
getDelegate().setSkipStateChangedEvent(false);
93+
}
9194
}
9295
}
9396

@@ -111,41 +114,63 @@ public int[] getSelectedIndexes() {
111114
@Override
112115
public void add(final String item, final int index) {
113116
synchronized (getDelegateLock()) {
114-
getDelegate().getModel().add(index, item);
115-
revalidate();
117+
getDelegate().setSkipStateChangedEvent(true);
118+
try {
119+
getDelegate().getModel().add(index, item);
120+
revalidate();
121+
} finally {
122+
getDelegate().setSkipStateChangedEvent(false);
123+
}
116124
}
117125
}
118126

119127
@Override
120128
public void delItems(final int start, final int end) {
121129
synchronized (getDelegateLock()) {
122-
getDelegate().getModel().removeRange(start, end);
123-
revalidate();
130+
getDelegate().setSkipStateChangedEvent(true);
131+
try {
132+
getDelegate().getModel().removeRange(start, end);
133+
revalidate();
134+
} finally {
135+
getDelegate().setSkipStateChangedEvent(false);
136+
}
124137
}
125138
}
126139

127140
@Override
128141
public void removeAll() {
129142
synchronized (getDelegateLock()) {
130-
getDelegate().getModel().removeAllElements();
131-
revalidate();
143+
getDelegate().setSkipStateChangedEvent(true);
144+
try {
145+
getDelegate().getModel().removeAllElements();
146+
revalidate();
147+
} finally {
148+
getDelegate().setSkipStateChangedEvent(false);
149+
}
132150
}
133151
}
134152

135153
@Override
136154
public void select(final int index) {
137155
synchronized (getDelegateLock()) {
138156
getDelegate().setSkipStateChangedEvent(true);
139-
getDelegate().getView().setSelectedIndex(index);
140-
getDelegate().setSkipStateChangedEvent(false);
157+
try {
158+
getDelegate().getView().setSelectedIndex(index);
159+
} finally {
160+
getDelegate().setSkipStateChangedEvent(false);
161+
}
141162
}
142163
}
143164

144165
@Override
145166
public void deselect(final int index) {
146167
synchronized (getDelegateLock()) {
147-
getDelegate().getView().getSelectionModel().
148-
removeSelectionInterval(index, index);
168+
getDelegate().setSkipStateChangedEvent(true);
169+
try {
170+
getDelegate().getView().removeSelectionInterval(index, index);
171+
} finally {
172+
getDelegate().setSkipStateChangedEvent(false);
173+
}
149174
}
150175
}
151176

test/jdk/ProblemList.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ java/awt/EventQueue/PushPopDeadlock/PushPopDeadlock.java 8024034 generic-all
150150
java/awt/grab/EmbeddedFrameTest1/EmbeddedFrameTest1.java 7080150 macosx-all
151151
java/awt/event/InputEvent/EventWhenTest/EventWhenTest.java 8168646 generic-all
152152
java/awt/List/KeyEventsTest/KeyEventsTest.java 8201307 linux-all
153+
java/awt/List/NoEvents/ProgrammaticChange.java 8201307 linux-all
153154
java/awt/Paint/ListRepaint.java 8201307 linux-all
154155
java/awt/Mixing/AWT_Mixing/HierarchyBoundsListenerMixingTest.java 8049405 macosx-all
155156
java/awt/Mixing/AWT_Mixing/OpaqueOverlapping.java 8370584 windows-x64
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/*
2+
* Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
import java.awt.Frame;
25+
import java.awt.List;
26+
import java.awt.Robot;
27+
import java.util.Arrays;
28+
import java.util.function.Supplier;
29+
30+
/**
31+
* @test
32+
* @bug 8371657
33+
* @key headful
34+
* @summary Checks that programmatic changes to a List do not fire events
35+
*/
36+
public final class ProgrammaticChange {
37+
38+
private static Robot robot;
39+
private static volatile boolean itemEvent;
40+
private static volatile boolean actionEvent;
41+
42+
public static void main(String[] args) throws Exception {
43+
robot = new Robot();
44+
robot.mouseMove(0, 0); // Just in case, the mouse may affect selection
45+
46+
var creators = Arrays.<Supplier<List>>asList(
47+
List::new, () -> createList(false), () -> createList(true)
48+
);
49+
for (Supplier<List> creator : creators) {
50+
test(creator, true); // Test displayable list
51+
test(creator, false); // Test non-displayable list
52+
}
53+
}
54+
55+
private static void test(Supplier<List> creator, boolean displayable) {
56+
List list = creator.get();
57+
list.addItemListener(event -> {
58+
System.err.println("event = " + event);
59+
itemEvent = true;
60+
});
61+
list.addActionListener(event -> {
62+
System.err.println("event = " + event);
63+
actionEvent = true;
64+
});
65+
66+
Frame frame = null;
67+
try {
68+
if (displayable) {
69+
frame = new Frame();
70+
frame.setSize(300, 200);
71+
frame.setLocationRelativeTo(null);
72+
frame.add(list);
73+
frame.setVisible(true);
74+
}
75+
tryTriggerEvents(list);
76+
} finally {
77+
if (frame != null) {
78+
frame.dispose();
79+
}
80+
}
81+
}
82+
83+
private static void tryTriggerEvents(List list) {
84+
// Only "select" and "deselect" should not fire events per the spec,
85+
// but we also check other methods to prevent accidental changes
86+
selectAll(list);
87+
verify();
88+
deselectAll(list);
89+
verify();
90+
91+
// "add" may change the current selection
92+
selectAll(list);
93+
list.add("newItemStart", 0);
94+
list.add("newItemMid", 1);
95+
list.add("newItemEnd");
96+
verify();
97+
98+
// "remove" may change the current selection
99+
selectAll(list);
100+
list.remove(0);
101+
verify();
102+
103+
// "makeVisible" may change the current selection
104+
for (int i = 0; i < 100; i++){
105+
list.add("newItem_" + i, 0);
106+
}
107+
selectAll(list);
108+
list.makeVisible(1);
109+
list.makeVisible(99);
110+
verify();
111+
112+
// "setMultipleMode" may change the current selection
113+
selectAll(list);
114+
list.setMultipleMode(!list.isMultipleMode());
115+
selectAll(list);
116+
list.setMultipleSelections(!list.allowsMultipleSelections());
117+
verify();
118+
119+
// "removeAll" may change the current selection
120+
selectAll(list);
121+
list.removeAll();
122+
verify();
123+
124+
// No extra logic; just calling methods to touch all code paths
125+
list.add("newItem1");
126+
list.getSelectedIndex();
127+
list.getSelectedIndexes();
128+
list.getSelectedItem();
129+
list.getSelectedItems();
130+
list.getSelectedObjects();
131+
list.getVisibleIndex();
132+
list.isIndexSelected(0);
133+
list.isSelected(0);
134+
135+
list.add("newItem2");
136+
list.delItems(0, 0);
137+
list.addItem("newItem4");
138+
list.delItem(0);
139+
list.addItem("newItem6", 0);
140+
list.replaceItem("newItem7", 0);
141+
list.remove("newItem7");
142+
list.add("newItem8");
143+
list.clear();
144+
verify();
145+
}
146+
147+
private static void selectAll(List list) {
148+
for (int index = 0; index < list.getItemCount(); index++) {
149+
list.select(index);
150+
}
151+
}
152+
153+
private static void deselectAll(List list) {
154+
for (int index = 0; index < list.countItems(); index++) {
155+
list.deselect(index);
156+
}
157+
}
158+
159+
private static List createList(boolean multipleMode) {
160+
List list = new List(4, multipleMode);
161+
for (String item : new String[]{"item1", "item2", "item3"}) {
162+
list.add(item);
163+
}
164+
return list;
165+
}
166+
167+
private static void verify() {
168+
robot.waitForIdle();
169+
robot.delay(700); // Large delay, we are waiting for unexpected events
170+
if (actionEvent || itemEvent) {
171+
System.err.println("itemEvent: " + itemEvent);
172+
System.err.println("actionEvent: " + actionEvent);
173+
throw new RuntimeException("Unexpected event");
174+
}
175+
}
176+
}

0 commit comments

Comments
 (0)