Hi Phil,
Thanks for guiding with the documentation. The fix is modified after a relook at the documentation, and observed two issues [mentioned below]. webrev: http://cr.openjdk.java.net/~arapte/8198000/webrev.02 Fix Justification: LB_ITEMFROMPOINT documentation(https://docs.microsoft.com/en-us/windows/desktop/controls/lb-itemfrompoint): Message LB_ITEMFROMPOINT returns a LONG value. There is no mention of LB_ERR as return value, like it is mentioned clearly for some of the other messages as LB_GETCOUNT, LB_SETITEMHEIGHT, LB_GETTEXT, LB_GETCURSEL. So the existing comparison against LB_ERR is incorrect. The two parts LOWORD and HIWORD of return value are of type WORD(unsigned short), so the return value can never be negative. It is another reason to not to compare the returned index value with LB_ERR which is defined as (-1). Fix: Extract the index from LOWORD into a WORD variable and verify only if the index is smaller than the list size. (webrev.02) This fixes both JDK-8198000 & below mentioned Issue 1. 2 other Issues: Issue 1: NON selection of an item. Currently the returned LONG value is used without extracting the LOWORD. As far as the HIWORD is 0, the LONG return value would be same as index of item(LOWORD). But when HIWORD is 1, the LONG return value would be a large unexpected value. If it is used as an index, then it would result in NON selection of the item. Test program: http://cr.openjdk.java.net/~arapte/8198000/ListSize3.java Steps: Compile and execute the test program with release build JDK. Click in the list's client area below the last item, i.e. do not click on any item. Expected behavior: Last item should get selected. Actual behavior: Last item does not get selected on first click. But the focused item gets selected after few clicks. Issue 2: Incorrect selection when list size exceeds sizeof(WORD) [0xFFFF]. Steps: Compile and execute the program with release build JDK (with or without the fix) : http://cr.openjdk.java.net/~arapte/8198000/List65544.java Click the first item in list, press End key. Click the last item 65544. Expected behavior: The item 65544 should get selected. Actual behavior: The item 65544 does not get selected and instead item 8 gets selected. Verified only windows behavior. I suggest to file a new JBS for this issue. Regards, Ambarish From: Phil Race Sent: Thursday, November 01, 2018 1:51 AM To: Ambarish Rapte <ambarish.ra...@oracle.com>; awt-dev@openjdk.java.net Subject: Re: <AWT Dev> [12] RFR : JDK-8198000 : java/awt/List/EmptyListEventTest/EmptyListEventTest.java debug assert on Windows That adds what I suggested, but I had also suggested you leave in what you had added as it also adds some protection. Additionally I read the MS docs and they do explain the 131071 return value. The message this code is sending is LB_ITEMFROMPOINT and the docs say here https://docs.microsoft.com/en-us/windows/desktop/controls/lb-itemfrompoint that >The return value contains the index of the nearest item in the HYPERLINK >"https://msdn.microsoft.com/library/windows/desktop/ms632659"LOWORD. The >HYPERLINK "https://msdn.microsoft.com/library/windows/desktop/ms632657"HIWORD >is zero if the > specified point is in the client area of the list box, or one >if it is outside the client area" You got 131071 which is, in hex 0X1FFFF. So you got "1" for hi-word, meaning "outside client area" and "-1" for loword, meaning the index. And a return index of "-1" doubtless means an error .. -phil. On 10/31/18 12:49 PM, Ambarish Rapte wrote: Hi Phil & Sergey, This issue was introduced with the fix for HYPERLINK "https://bugs.openjdk.java.net/browse/JDK-6806217"JDK-6806217, in 7b55, which modified AwtList::HandleEvent(), so It was not observed with JDK6. Please review the updated change as discussed offline: http://cr.openjdk.java.net/~arapte/8198000/webrev.01/ Regards, Ambarish From: Phil Race Sent: Tuesday, October 30, 2018 2:09 AM To: Ambarish Rapte HYPERLINK "mailto:ambarish.ra...@oracle.com";<ambarish.ra...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: Re: <AWT Dev> [12] RFR : JDK-8198000 : java/awt/List/EmptyListEventTest/EmptyListEventTest.java debug assert on Windows On 10/29/18 7:31 AM, Ambarish Rapte wrote: Hi, Please review this Windows only fix, JBS: https://bugs.openjdk.java.net/browse/JDK-8198000 Webrev: http://cr.openjdk.java.net/~arapte/8198000/webrev.00/ Issue: Test asserts with debug build jdk, only on windows. Only the debug build turns on asserts. But I think JDK 6 always turned on asserts, and this test was introduced in 6, so something must have changed else we'd have seen this test fail a long time ago. Can you identify what it was ? Also same comment as the other bug - you need to add the bug id to the test. Assert at Line no 77, awt_List.h :: IsItemSelected() awt_List.cpp :: AwtList::HandleEvent() calls IsItemSelected() with an incorrect index value. Why ? In AwtList::HandleEvent() , the call SendListMessage(LB_ITEMFROMPOINT, 0, msg->lParam) returns an arbitrary value 131071, which gets passed to IsItemSelected(). Could not find any relevance to the value 131071, from LB_ITEMFROMPOINT doc. That is (128*1024)-1, so it is probably not "arbitrary". Please add the eval above to the bug report .. once we have a complete understanding. -phil. Fix: Index should be verified before making call to IsItemSelected() : http://cr.openjdk.java.net/~arapte/8198000/webrev.00/ Verification: All list tests pass. Regards, Ambarish
