<AWT Dev> 7171412: awt Choice doesn't fire ItemStateChange when selecting item after select() call
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: tim_english at hotmail.com; awt-dev at openjdk.java.net
> Subject: Re: <AWT Dev> 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 '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 m_selectedItem when select(int) called
> void AwtChoice::_Select(void *param)
> {
> JNIEnv *env = (JNIEnv *)JNU_GetEnv(jvm, JNI_VERSION_1_2);
> SelectStruct *ss = (SelectStruct *)param;
> jobject choice = ss->choice;
> jint index = ss->index;
> AwtChoice *c = NULL;
> PDATA pData;
> JNI_CHECK_PEER_GOTO(choice, done);
> c = (AwtChoice *)pData;
> if (::IsWindow(c->GetHWnd()))
> {
> c->SendMessage(CB_SETCURSEL, index);
> + m_selectedItem = index;
> // c->VerifyState();
> }
> done:
> env->DeleteGlobalRef(choice);
> delete ss;
> }
> Possible Fix 2 Eliminate m_selectedItem 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 m_selectedItem 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 == CBN_SELCHANGE) {
> int selectedItem = (int)SendMessage(CB_GETCURSEL);
> + // tim_english: not sure if it is legal to call from here
> + JNIEnv *env = (JNIEnv *)JNU_GetEnv(jvm, JNI_VERSION_1_2);
> + jobject target = GetTarget(env);
> + int javaIndex = JNU_CallMethodByName(env, NULL, target,
> "getSelectedIndex", "()I").i;
> + env->DeleteLocalRef(target);
>
> + if (selectedItem != CB_ERR && javaIndex != selectedItem){
> - if (selectedItem != CB_ERR && m_selectedItem !=
> selectedItem){
> - m_selectedItem = selectedItem;
> DoCallback("handleAction", "(I)V", selectedItem);
> }
> } else if (notifyCode == CBN_DROPDOWN) {
>
>
>
> 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