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

Oleg Pekhovskiy oleg.pekhovskiy at oracle.com
Wed Aug 15 15:52:29 PDT 2012

Hi Tim,

please see my comments below...

Thanks, Oleg

15.08.2012 23:11, Tim English wrote:

Thanks for the reply, Oleg.

Should I try jumping on jdk7u8? I first started with jdk8, but following the procedures for it seemed like there were lots of pieces/scripts that were missing from jdk8. The rule is simple: if fixes to jdk8 and jdk7u are different you prepare them separately, if they are equal you prepare the fix to jdk8 first, then do backport. http://openjdk.java.net/contribute/ (5. Know what to expect, last paragraph).

I was thinking that Fix 1 would be appropriate for a jdk7u implementation as a patch. Merge it into 8 (or patch 8 with Fix 1 -> 7uN) but then supersede it in 8 by something along the lines of Fix 2. Since the 7 code is already stable, future maintenance concerns that Fix 2 address would not be as critical in 7. If new code is ever added to 8 or later, the strategy of Fix 2 should reduce other regressions. If your rating is "crash" == priority High, I agree this would be Medium on your radar. On our internal rating scale, anything that will insert incorrect data into the database is what we consider "Show Stopper". This falls in our "Show Stopper" category since the user might might be looking at "A" on the windows control, but the Java control just /knows /that the value is"B". "B" gets inserted into the DB. Fail. I just ran a query and there are over 1100 Choice fields in our codebase. I agree that for your application that could be "show stopper", But the fix for 6770017 was pushed before jdk7 was released. It means that such behavior is "normal" for jdk7 and its updates. In other words somebody could rely on such behavior having workaround and thus by fixing that we could break its logic. Nevertheless, I'm not the person who make the final decision on issue's priority :)

Slightly different topic around gmake:

I would try to get a regression for this (using the "robot" API?), Yes, you should use robot for that.

but I could never get the "sanity" target to build cleanly. That is also the reason I have not tried committing (to the "gate"?). I am positive (have you ever heard that before?), that Fix 1 is safe and will fix 7171412, but it would be nice to see it work! If I could have compiled it and confirmed the change, I probably would have just emailed the fix in and hope someone else would commit it. Just review the boundary points between java and the windows control via the SendMessage calls; they are nice and isolated.

I think my cygwin is too new and that is why make is failing. Some conlicft in the gmake system where windows drive letter colons are confusing the make parser. *Is there a mercurial repository where they keep the preferred versions of cygwin, etc used in the build process?* (open source tools of course, we wouldn't want to check in VS2010)**The minimum cygwin version is specified in the docs, but the current cygwin is far beyond that older version. I will have to try to hunt up an older version. I tried remapping everything via cygdrive to work around the colons, but there were still a few references I was having trouble shaking. I think it was from some derived names.

I have been leaving the sanityCheckMessages.txt open in my desktop to remind me of this. I suspect that TOPDIR(.) is resolving to S:\jdk7u6 when it is substituted, when I need . to resolve to /cygdrive/s/jdk7u6. Build Directory Structure: CWD = /cygdrive/s/jdk7u6 TOPDIR = *. * I strongly recommend you to read "OpenJDK Build README" from here: http://hg.openjdk.java.net/jdk7/build/raw-file/tip/README-builds.html Pay attention to the notes about GNU Make 3.81 usage (it could be downloaded from here http://ftp.gnu.org/pub/gnu/make/ ).

I also advice you to build just jdk workspace (http://hg.openjdk.java.net/jdk7u/jdk7u/jdk) and use ALT_JDK_IMPORT_PATH with the path to the latest jdk8 (http://jdk8.java.net/download.html) or jdk7u (http://jdk7.java.net/download.html) build.

I stopped after 3 or 4 days, because I have to work on some of my own code "opportunities" before I will get a chance to try again. Hopefully, I will get back to the gmake/cygwin stuffit next week.

Thanks, Tim

*I am a newb to committing for JDK, not to Java nor programming. I have been using Java since 1.0, and I used to program WinAPI starting with 2.1 and stopping after NT 4.0. Seriously, the WinAPI really has NOT changed much since 2.1, Protected Memory and Combo Boxes were radical advances for 3.0. You could commit to your local repository but you couldn't push to hg.openjdk.java.net. There are rules to become a committer ("pusher" indeed): http://openjdk.java.net/projects/

*From the rash of emails the last few weeks, I can tell that you all have been BUSY! *Funny story here... while updating from mercurial, McAfee clobbered a test jar as an "invalid format" that was commited as part of a regression test that ....drum roll, please... checks for "invalid format". I don't think this is considered a PASS since the test case won't find the bad jar! lol ------------------------------------------------------------------------ Date: Wed, 15 Aug 2012 00:09:20 +0400 From: oleg.pekhovskiy at oracle.com To: timenglish at hotmail.com; awt-dev at openjdk.java.net Subject: Re: 7171412: awt Choice doesn't fire ItemStateChange when selecting item after select() call 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 'mselectedItem' 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 CBNCHANGE 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/20120816/112f5595/attachment.html

More information about the awt-dev mailing list