7171412: awt Choice doesn't fire ItemStateChange when selecting item after select() call (original) (raw)

Artem Ananiev artem.ananiev at oracle.com
Thu Aug 16 09:57:11 PDT 2012

Hi, Tim,

see my comments below.

On 7/25/2012 1:31 AM, Tim English wrote:

newb here.. Trying to follow procedure and run this by awt-dev before I dive into this. I ran into 7171412, produced a similar test case to the one in the bug report, and then found the existing bug report. I consider this High priority since any GUI that uses an awt Choice could be showing the end user the wrong information if the GUI is counting on the ItemListener for correct state.

I don't say counting on ItemEvents is wrong, but application developers should be careful here. And the problem is with missing events on Choice.select() calls.

The code in AwtChoice::WmNotify looks like it is trying to prevent notifications if the user keeps selecting the same item over and over.

Correct.

I came across a bug/feature request along those lines when searching for 7171412. Possible Fix 1 update mselectedItem when select(int) called void AwtChoice::Select(void *param) { JNIEnv *env = (JNIEnv *)JNUGetEnv(jvm, JNIVERSION12); SelectStruct *ss = (SelectStruct *)param; jobject choice = ss->choice; jint index = ss->index; AwtChoice *c = NULL; PDATA pData; JNICHECKPEERGOTO(choice, done); c = (AwtChoice *)pData; if (::IsWindow(c->GetHWnd())) { c->SendMessage(CBSETCURSEL, index); + mselectedItem = index; // c->VerifyState(); } done: env->DeleteGlobalRef(choice); delete ss; }

This fix #1 looks fine.

Possible Fix 2 Eliminate mselectedItem and replace it with java object value Only notify java if java's selected index does not match the Windows control selected index This eliminates the mselectedItem so we only have 2 copies of the state instead of 3. It is probably more efficient to hit the java field, directly, but I have not researched jfieldID. This is just a first stab. MsgRouting AwtChoice::WmNotify(UINT notifyCode) { if (notifyCode == CBNSELCHANGE) { int selectedItem = (int)SendMessage(CBGETCURSEL); + // timenglish: not sure if it is legal to call from here + JNIEnv *env = (JNIEnv *)JNUGetEnv(jvm, JNIVERSION12); + jobject target = GetTarget(env); + int javaIndex = JNUCallMethodByName(env, NULL, target, "getSelectedIndex", "()I").i; + env->DeleteLocalRef(target);

+ if (selectedItem != CBERR && javaIndex != selectedItem){ - if (selectedItem != CBERR && mselectedItem != selectedItem){ - mselectedItem = selectedItem; DoCallback("handleAction", "(I)V", selectedItem); } } else if (notifyCode == CBNDROPDOWN) {

Reading the field value directly in WM_NOTIFY handler is also possible, but #1 seems more simple to me.

http://hg.openjdk.java.net/jdk8/awt/jdk/rev/3a2355dcef13

http://hg.openjdk.java.net/jdk7u/jdk7u6/jdk/rev/3a2355dcef13 Any advice is appreciated.. Tim

Since you're not OpenJDK Committer, you'll need to find a sponsor to push your fix to the workspace. Oleg or I can do this, if you don't mind.

The fix for JDK8 don't require any additional approvals, but 7u8 process is more complex: explicit request to include a fix to 7u must be sent to jdk7u-dev alias [1]. But first of all, the fix should be pushed to JDK8 (see Rule 1 at [2]), so let's start with JDK8 and then proceed to 7u8.

[1] http://openjdk.java.net/projects/jdk7u/approval-template.html [2] http://openjdk.java.net/projects/jdk7u/groundrules.html

Thanks,

Artem

More information about the awt-dev mailing list