7171412: awt Choice doesn't fire ItemStateChange when selecting item after select() call (original) (raw)
Oleg Pekhovskiy oleg.pekhovskiy at oracle.com
Tue Aug 14 13:09:20 PDT 2012
- Previous message: [8] Review request for 7172187 [macosx] JAWT native CALayer not positioned over Canvas
- Next message: hg: jdk8/awt/jdk: 7190813: (launcher) RPATH needs to have additional paths
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Tim,
sorry for long-awaited answer, thank you for your work and here are my comments...
Reviewing the code related to 7171412 I found another issue 6770017 where 'm_selectedItem' was added. The purpose of that variable is to avoid surplus sending of ItemEvent when the user selects the same item once again (combo-box send CBN_CHANGE on Windows XP each time regardless the index chosen). From the other side mirroring index variable is not a clear solution. So your second version of fix is more suitable.
Retrieving field value is better by performance but requires more changes like implementation of 'initIDs()' native method and calling it from Choice static section (when in non-headless mode) to initialize 'jfieldID' for 'selectedIndex'.
7171412 has 'medium' priority that is enough for the issue that touches a specific behavior of one component only and is not so critical as crash or deadlock.
It would be nice to have an automatic regression test for this issue :)
Thanks, Oleg
25.07.2012 1:31, 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. The code in AwtChoice::WmNotify looks like it is trying to prevent notifications if the user keeps selecting the same item over and over. 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; } 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) {
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
-------------- next part -------------- An HTML attachment was scrubbed... URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20120815/e69e4581/attachment.html
- Previous message: [8] Review request for 7172187 [macosx] JAWT native CALayer not positioned over Canvas
- Next message: hg: jdk8/awt/jdk: 7190813: (launcher) RPATH needs to have additional paths
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]