[12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Shashi, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/ As for the test, it has already been corrected and pushed in JDK-8197810. Thanks, Krishna From: Shashidhara Veerabhadraiah Sent: Monday, July 2, 2018 11:30 PM To: Krishna Addepalli ; awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Krishna, The changes looks fine. Please don't forget to add the bug id to the test before the push. And add this bug JDK-8197810 "as relate to" the current bug(in bug db). Thanks and regards, Shashi From: Krishna Addepalli Sent: Tuesday, July 3, 2018 8:00 AM To: Shashidhara Veerabhadraiah ; awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Shashi, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/ As for the test, it has already been corrected and pushed in JDK-8197810. Thanks, Krishna From: Shashidhara Veerabhadraiah Sent: Monday, July 2, 2018 11:30 PM To: Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Can we use existing 'skipPostMessage' member for this check instead of introducing additional member in LWChoicePeer class? From: Shashidhara Veerabhadraiah Sent: Tuesday, July 03, 2018 12:16 PM To: Krishna Addepalli; awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, The changes looks fine. Please don't forget to add the bug id to the test before the push. And add this bug JDK-8197810 "as relate to" the current bug(in bug db). Thanks and regards, Shashi From: Krishna Addepalli Sent: Tuesday, July 3, 2018 8:00 AM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Shashi, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/ As for the test, it has already been corrected and pushed in JDK-8197810. Thanks, Krishna From: Shashidhara Veerabhadraiah Sent: Monday, July 2, 2018 11:30 PM To: Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Ajit, I don't think we can use 'skipPostMessage' for this check for the following reasons: 1. 'skipPostMessage' is intended to filter out item unselected messages, which are posted in case of JComboBox. 2. I need to store the previously selected index to compare against the current index, and I'll need to change the skipPostMessage to integer type - although doable, will be a maintenance headache. 3. As much as I have seen, skipPostMessage variable is restored to false in the same function call (select and remove), whereas for selectedIndex, this won't be the case. 4. Also, remove function is not called from here, but from outside. So, the state will clash, when I save the last selected index, and immediately remove gets called. Hope this clarifies. Thanks, Krishna From: Ajit Ghaisas Sent: Tuesday, July 3, 2018 1:15 PM To: Shashidhara Veerabhadraiah ; Krishna Addepalli ; awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Can we use existing 'skipPostMessage' member for this check instead of introducing additional member in LWChoicePeer class? From: Shashidhara Veerabhadraiah Sent: Tuesday, July 03, 2018 12:16 PM To: Krishna Addepalli; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, The changes looks fine. Please don't forget to add the bug id to the test before the push. And add this bug JDK-8197810 "as relate to" the current bug(in bug db). Thanks and regards, Shashi From: Krishna Addepalli Sent: Tuesday, July 3, 2018 8:00 AM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Shashi, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/ As for the test, it has already been corrected and pushed in JDK-8197810. Thanks, Krishna From: Shashidhara Veerabhadraiah Sent: Monday, July 2, 2018 11:30 PM To: Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Krishna, Thanks for the explanation. Changes look good. You will need a 'R' reviewer to review this though. Regards, Ajit From: Krishna Addepalli Sent: Tuesday, July 03, 2018 2:20 PM To: Ajit Ghaisas; Shashidhara Veerabhadraiah; awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Ajit, I don't think we can use 'skipPostMessage' for this check for the following reasons: 1. 'skipPostMessage' is intended to filter out item unselected messages, which are posted in case of JComboBox. 2. I need to store the previously selected index to compare against the current index, and I'll need to change the skipPostMessage to integer type - although doable, will be a maintenance headache. 3. As much as I have seen, skipPostMessage variable is restored to false in the same function call (select and remove), whereas for selectedIndex, this won't be the case. 4. Also, remove function is not called from here, but from outside. So, the state will clash, when I save the last selected index, and immediately remove gets called. Hope this clarifies. Thanks, Krishna From: Ajit Ghaisas Sent: Tuesday, July 3, 2018 1:15 PM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Can we use existing 'skipPostMessage' member for this check instead of introducing additional member in LWChoicePeer class? From: Shashidhara Veerabhadraiah Sent: Tuesday, July 03, 2018 12:16 PM To: Krishna Addepalli; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, The changes looks fine. Please don't forget to add the bug id to the test before the push. And add this bug JDK-8197810 "as relate to" the current bug(in bug db). Thanks and regards, Shashi From: Krishna Addepalli Sent: Tuesday, July 3, 2018 8:00 AM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Shashi, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/ As for the test, it has already been corrected and pushed in JDK-8197810. Thanks, Krishna From: Shashidhara Veerabhadraiah Sent: Monday, July 2, 2018 11:30 PM To: Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Prasanta/Sergey, Could you review this ? @Ajit, Shashi - Thanks for your review. Thanks, Krishna From: Ajit Ghaisas Sent: Tuesday, July 3, 2018 3:31 PM To: Krishna Addepalli ; Shashidhara Veerabhadraiah ; awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Thanks for the explanation. Changes look good. You will need a 'R' reviewer to review this though. Regards, Ajit From: Krishna Addepalli Sent: Tuesday, July 03, 2018 2:20 PM To: Ajit Ghaisas; Shashidhara Veerabhadraiah; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Ajit, I don't think we can use 'skipPostMessage' for this check for the following reasons: 1. 'skipPostMessage' is intended to filter out item unselected messages, which are posted in case of JComboBox. 2. I need to store the previously selected index to compare against the current index, and I'll need to change the skipPostMessage to integer type - although doable, will be a maintenance headache. 3. As much as I have seen, skipPostMessage variable is restored to false in the same function call (select and remove), whereas for selectedIndex, this won't be the case. 4. Also, remove function is not called from here, but from outside. So, the state will clash, when I save the last selected index, and immediately remove gets called. Hope this clarifies. Thanks, Krishna From: Ajit Ghaisas Sent: Tuesday, July 3, 2018 1:15 PM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Can we use existing 'skipPostMessage' member for this check instead of introducing additional member in LWChoicePeer class? From: Shashidhara Veerabhadraiah Sent: Tuesday, July 03, 2018 12:16 PM To: Krishna Addepalli; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, The changes looks fine. Please don't forget to add the bug id to the test before the push. And add this bug JDK-8197810 "as relate to" the current bug(in bug db). Thanks and regards, Shashi From: Krishna Addepalli Sent: Tuesday, July 3, 2018 8:00 AM To: Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Shashi, Thanks for the review. Here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/ As for the test, it has already been corrected and pushed in JDK-8197810. Thanks, Krishna From: Shashidhara Veerabhadraiah Sent: Monday, July 2, 2018 11:30 PM To: Krishna Addepalli mailto:krishna.addepa...@oracle.com"krishna.addepa...@oracle.com>; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: RE: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi Krishna, Below are the comments on the fix that you made. 1. Based on the getSelectedIndex(), I think the selectedIndex should be initialized to -1. 0 seems to be a valid value. 2. A test may be required to test out the behavior because as per the comment in the bug, the test SelecteCurrentItemTest.html should fail now on all platforms since the passing platforms software was changed. Other than that the changes looks fine. Thanks and regards, shashi From: Krishna Addepalli Sent: Monday, July 2, 2018 3:10 PM To: HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi All, Please review a fix for JDK-8014503: https://bugs.openjdk.java.net/browse/JDK-8014503 Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/ On Windows, the default behavior has been changed to not send an ItemEvent, if the same item is selected again, whereas this was not changed for Mac and Linux. This fix changes that behavior, and makes it consistent for all the three platforms. Thanks, Krishna
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi, Krishna
If I read the code in LWChoicePeer.java file correctly, then you can fix
the problem by dropping the method below in JComboBoxDelegate class:
/**
* We should post ITEM_STATE_CHANGED event when the same element is
* reselected.
*/
@Override
public void setSelectedItem(final Object anObject) {
Are you sure that storing the index of the selected element in
XChoicePeer class is enough? Because the element itself at this index
may change.
On 04/07/2018 13:48, Krishna Addepalli wrote:
Hi Prasanta/Sergey,
Could you review this ?
@Ajit, Shashi – Thanks for your review.
Thanks,
Krishna
*From:* Ajit Ghaisas
*Sent:* Tuesday, July 3, 2018 3:31 PM
*To:* Krishna Addepalli ; Shashidhara
Veerabhadraiah ;
awt-dev@openjdk.java.net
*Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
implementation should be made consistent across platforms.
Hi Krishna,
Thanks for the explanation. Changes look good.
You will need a ‘R’ reviewer to review this though.
Regards,
Ajit
*From:* Krishna Addepalli
*Sent:* Tuesday, July 03, 2018 2:20 PM
*To:* Ajit Ghaisas; Shashidhara Veerabhadraiah; awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>
*Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
implementation should be made consistent across platforms.
Hi Ajit,
I don’t think we can use ‘skipPostMessage’ for this check for the
following reasons:
1.‘skipPostMessage’ is intended to filter out item unselected messages,
which are posted in case of JComboBox.
2.I need to store the previously selected index to compare against the
current index, and I’ll need to change the skipPostMessage to integer
type – although doable, will be a maintenance headache.
3.As much as I have seen, skipPostMessage variable is restored to false
in the same function call (select and remove), whereas for
selectedIndex, this won’t be the case.
4.Also, remove function is not called from here, but from outside. So,
the state will clash, when I save the last selected index, and
immediately remove gets called.
Hope this clarifies.
Thanks,
Krishna
*From:* Ajit Ghaisas
*Sent:* Tuesday, July 3, 2018 1:15 PM
*To:* Shashidhara Veerabhadraiah <mailto:shashidhara.veerabhadra...@oracle.com>>; Krishna Addepalli
mailto:krishna.addepa...@oracle.com>>;
awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
*Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
implementation should be made consistent across platforms.
Can we use existing ‘skipPostMessage’ member for this check instead of
introducing additional member in LWChoicePeer class?
*From:* Shashidhara Veerabhadraiah
*Sent:* Tuesday, July 03, 2018 12:16 PM
*To:* Krishna Addepalli; awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>
*Subject:* Re: [12]RFR : JDK-8014503: AWT Choice
implementation should be made consistent across platforms.
Hi Krishna, The changes looks fine.
Please don’t forget to add the bug id to the test before the push. And
add this bug JDK-8197810 “as relate to” the current bug(in bug db).
Thanks and regards,
Shashi
*From:* Krishna Addepalli
*Sent:* Tuesday, July 3, 2018 8:00 AM
*To:* Shashidhara Veerabhadraiah <mailto:shashidhara.veerabhadra...@oracle.com>>;
awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
*Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
implementation should be made consistent across platforms.
Hi Shashi,
Thanks for the review. Here is the updated webrev:
http://cr.openjdk.java.net/~kaddepalli/8014503/webrev01/
As for the test, it has already been corrected and pushed in JDK-8197810.
Thanks,
Krishna
*From:* Shashidhara Veerabhadraiah
*Sent:* Monday, July 2, 2018 11:30 PM
*To:* Krishna Addepalli <mailto:krishna.addepa...@oracle.com>>; awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>
*Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
implementation should be made consistent across platforms.
Hi Krishna, Below are the comments on the fix that you made.
1.Based on the getSelectedIndex(), I think the selectedIndex should be
initialized to -1. 0 seems to be a valid value.
2.A test may be required to test out the behavior because as per the
comment in the bug, the test SelecteCurrentItemTest.html should fail now
on all platforms since the passing platforms software was changed.
Other than that the changes looks fine.
Thanks and regards,
shashi
*From:* Krishna Addepalli
*Sent:* Monday, July 2, 2018 3:10 PM
*To:* awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
*Subject:* [12]RFR : JDK-8014503: AWT Choice implementation
should be made consistent across platforms.
Hi All,
Please review a fix for JDK-8014503:
https://bugs.openjdk.java.net/browse/JDK-8014503
Webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev00/
On Windows, the default behavior has been changed to not send an
ItemEvent, if the same item is selected again
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Sergey,
The fix I propose is modeled after the code in Windows, which tracks items by
their selected index. The code in Mac is storing currently selected object,
which could raise the question about which object to select next, when the
selected object is removed/changed.
With index based approach, I see that, the index remains unchanged, even if the
object at that location is removed/changed, unless it causes the index to be
out of bounds.
This behavior is the same across Windows, Linux and Mac.
For your reference, below is the code I have used to observe this behavior:
import java.awt.Choice;
import java.awt.Frame;
import java.awt.EventQueue;
public class ChoiceExample
{
private static Choice c;
private static Frame f;
ChoiceExample(){
f= new Frame();
c = new Choice();
c.setBounds(100,100, 75,75);
c.add("Item 1");
c.add("Item 2");
c.add("Item 3");
c.add("Item 4");
c.add("Item 5");
f.add(c);
f.setSize(400,400);
f.setLayout(null);
f.setVisible(true);
}
private static void sleep(int ms) {
try {
Thread.sleep(ms);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
public static void main(String args[]) throws Exception
{
EventQueue.invokeAndWait(() -> {
ChoiceExample ce = new ChoiceExample();
System.out.println(c.getSelectedIndex());
sleep(1);
c.remove("Item 1");
System.out.println(c.getSelectedIndex());
sleep(1);
//c.select(c.getItemCount() - 1);
c.select("Item 5");
System.out.println(c.getSelectedIndex());
f.repaint();
sleep(5);
c.remove("Item 4");
System.out.println(c.getSelectedIndex());
f.repaint();
});
sleep(5000);
EventQueue.invokeAndWait(() -> {
f.dispose();
});
}
}
Thanks,
Krishna
-Original Message-
From: Sergey Bylokhov
Sent: Thursday, July 5, 2018 8:10 PM
To: Krishna Addepalli ; Ajit Ghaisas
; Shashidhara Veerabhadraiah
; awt-dev@openjdk.java.net
Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should
be made consistent across platforms.
Hi, Krishna
If I read the code in LWChoicePeer.java file correctly, then you can fix the
problem by dropping the method below in JComboBoxDelegate class:
/**
* We should post ITEM_STATE_CHANGED event when the same element is
* reselected.
*/
@Override
public void setSelectedItem(final Object anObject) {
Are you sure that storing the index of the selected element in XChoicePeer
class is enough? Because the element itself at this index may change.
On 04/07/2018 13:48, Krishna Addepalli wrote:
> Hi Prasanta/Sergey,
>
> Could you review this ?
>
> @Ajit, Shashi - Thanks for your review.
>
> Thanks,
>
> Krishna
>
> *From:* Ajit Ghaisas
> *Sent:* Tuesday, July 3, 2018 3:31 PM
> *To:* Krishna Addepalli ; Shashidhara
> Veerabhadraiah ;
> awt-dev@openjdk.java.net
> *Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
> implementation should be made consistent across platforms.
>
> Hi Krishna,
>
> Thanks for the explanation. Changes look good.
>
> You will need a 'R' reviewer to review this though.
>
> Regards,
>
> Ajit
>
> *From:* Krishna Addepalli
> *Sent:* Tuesday, July 03, 2018 2:20 PM
> *To:* Ajit Ghaisas; Shashidhara Veerabhadraiah;
> awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net>
> *Subject:* RE: [12]RFR : JDK-8014503: AWT Choice
> implementation should be made consistent across platforms.
>
> Hi Ajit,
>
> I don't think we can use 'skipPostMessage' for this check for the
> following reasons:
>
> 1.'skipPostMessage' is intended to filter out item unselected
> messages, which are posted in case of JComboBox.
>
> 2.I need to store the previously selected index to compare against the
> current index, and I'll need to change the skipPostMessage to integer
> type - although doable, will be a maintenance headache.
>
> 3.As much as I have seen, skipPostMessage variable is restored to
> false in the same function call (select and remove), whereas for
> selectedIndex, this won't be the case.
>
> 4.Also, remove function is not called from here, but from outside. So,
> the state will clash, when I save the last selected index, and
> immediately remove gets called.
>
> Hope this clarifies.
>
> Thanks,
>
> Krishna
>
> *From:* Ajit Ghaisas
> *Sent:* Tuesday, July 3, 2018 1:15 PM
> *To:* Shashidhar
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
On 13/08/2018 11:06, Krishna Addepalli wrote: The fix I propose is modeled after the code in Windows, which tracks items by their selected index. The code in Mac is storing currently selected object, which could raise the question about which object to select next, when the selected object is removed/changed. With index based approach, I see that, the index remains unchanged, even if the object at that location is removed/changed, unless it causes the index to be out of bounds. This behavior is the same across Windows, Linux and Mac. I guess behavior is the same, because of the shared code in Choice->remove->removeNoInvalidate which selects the new element if the old selected element was removed. And it is not affected by the code changed in the fix. -- Best regards, Sergey.
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Yes, the behavior is not because of the fix I made. I was trying to understand the implications of the index based selection approach, and now it looks to be consistent on all the platforms. On Mac, as per your suggestion, we can get the same behavior by removing the "setSelectedItem", but my question is, should I make this change or keep the index based change, since that would keep the implementation also consistent across platforms. Thanks, Krishna -Original Message- From: Sergey Bylokhov Sent: Tuesday, August 14, 2018 7:52 AM To: Krishna Addepalli ; Ajit Ghaisas ; Shashidhara Veerabhadraiah ; awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. On 13/08/2018 11:06, Krishna Addepalli wrote: > The fix I propose is modeled after the code in Windows, which tracks items by > their selected index. The code in Mac is storing currently selected object, > which could raise the question about which object to select next, when the > selected object is removed/changed. > With index based approach, I see that, the index remains unchanged, even if > the object at that location is removed/changed, unless it causes the index to > be out of bounds. > This behavior is the same across Windows, Linux and Mac. I guess behavior is the same, because of the shared code in Choice->remove->removeNoInvalidate which selects the new element if the old selected element was removed. And it is not affected by the code changed in the fix. -- Best regards, Sergey.
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi, Krishna. On 20/08/2018 04:30, Krishna Addepalli wrote: Yes, the behavior is not because of the fix I made. I was trying to understand the implications of the index based selection approach, and now it looks to be consistent on all the platforms. On Mac, as per your suggestion, we can get the same behavior by removing the "setSelectedItem", but my question is, should I make this change or keep the index based change, since that would keep the implementation also consistent across platforms. Can you please provide an example which will show the difference between these two solutions. btw initially the code in setSelectedItem(..) was added to fix the opposite bug. And current fix will skips some events which are generated to fix the opposite bug, it looks strange. Thanks, Krishna -Original Message- From: Sergey Bylokhov Sent: Tuesday, August 14, 2018 7:52 AM To: Krishna Addepalli ; Ajit Ghaisas ; Shashidhara Veerabhadraiah ; awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. On 13/08/2018 11:06, Krishna Addepalli wrote: The fix I propose is modeled after the code in Windows, which tracks items by their selected index. The code in Mac is storing currently selected object, which could raise the question about which object to select next, when the selected object is removed/changed. With index based approach, I see that, the index remains unchanged, even if the object at that location is removed/changed, unless it causes the index to be out of bounds. This behavior is the same across Windows, Linux and Mac. I guess behavior is the same, because of the shared code in Choice->remove->removeNoInvalidate which selects the new element if the old selected element was removed. And it is not affected by the code changed in the fix. -- Best regards, Sergey.
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Sergey, I have not been able to find any example of different behaviour between index based and object based logic, so I have made the changes on Mac as you suggested. Also, on Linux, I found that the Choice.java class holds a selectedIndex already, so I removed it in XChoicePeer.java, and tested it. Here is the new webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev03/ <http://cr.openjdk.java.net/~kaddepalli/8014503/webrev03/> Thanks, Krishna > On 23-Aug-2018, at 4:34 PM, Sergey Bylokhov > wrote: > > Hi, Krishna. > On 20/08/2018 04:30, Krishna Addepalli wrote: >> Yes, the behavior is not because of the fix I made. I was trying to >> understand the implications of the index based selection approach, and now >> it looks to be consistent on all the platforms. >> On Mac, as per your suggestion, we can get the same behavior by removing the >> "setSelectedItem", but my question is, should I make this change or keep the >> index based change, since that would keep the implementation also consistent >> across platforms. > > Can you please provide an example which will show the difference between > these two solutions. > > btw initially the code in setSelectedItem(..) was added to fix the opposite > bug. And current fix will skips some events which are generated to fix the > opposite bug, it looks strange. > >> Thanks, >> Krishna >> -Original Message- >> From: Sergey Bylokhov >> Sent: Tuesday, August 14, 2018 7:52 AM >> To: Krishna Addepalli ; Ajit Ghaisas >> ; Shashidhara Veerabhadraiah >> ; awt-dev@openjdk.java.net >> Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation >> should be made consistent across platforms. >> On 13/08/2018 11:06, Krishna Addepalli wrote: >>> The fix I propose is modeled after the code in Windows, which tracks items >>> by their selected index. The code in Mac is storing currently selected >>> object, which could raise the question about which object to select next, >>> when the selected object is removed/changed. >>> With index based approach, I see that, the index remains unchanged, even if >>> the object at that location is removed/changed, unless it causes the index >>> to be out of bounds. >>> This behavior is the same across Windows, Linux and Mac. >> I guess behavior is the same, because of the shared code in >> Choice->remove->removeNoInvalidate >> which selects the new element if the old selected element was removed. >> And it is not affected by the code changed in the fix. > > > -- > Best regards, Sergey.
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi, Krishna. Why did you update the test? Before the fix the robot used the middle of the first element, but after the fix it will click on the first pixel of the first element(it can missclick on the choice itself), not sure it will be stable enough. On 21/09/2018 15:30, Krishna Addepalli wrote: Hi Sergey, I have not been able to find any example of different behaviour between index based and object based logic, so I have made the changes on Mac as you suggested. Also, on Linux, I found that the Choice.java class holds a selectedIndex already, so I removed it in XChoicePeer.java, and tested it. Here is the new webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev03/ <http://cr.openjdk.java.net/%7Ekaddepalli/8014503/webrev03/> Thanks, Krishna On 23-Aug-2018, at 4:34 PM, Sergey Bylokhov mailto:sergey.bylok...@oracle.com>> wrote: Hi, Krishna. On 20/08/2018 04:30, Krishna Addepalli wrote: Yes, the behavior is not because of the fix I made. I was trying to understand the implications of the index based selection approach, and now it looks to be consistent on all the platforms. On Mac, as per your suggestion, we can get the same behavior by removing the "setSelectedItem", but my question is, should I make this change or keep the index based change, since that would keep the implementation also consistent across platforms. Can you please provide an example which will show the difference between these two solutions. btw initially the code in setSelectedItem(..) was added to fix the opposite bug. And current fix will skips some events which are generated to fix the opposite bug, it looks strange. Thanks, Krishna -Original Message- From: Sergey Bylokhov Sent: Tuesday, August 14, 2018 7:52 AM To: Krishna Addepalli <mailto:krishna.addepa...@oracle.com>>; Ajit Ghaisas mailto:ajit.ghai...@oracle.com>>; Shashidhara Veerabhadraiah <mailto:shashidhara.veerabhadra...@oracle.com>>; awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net> Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. On 13/08/2018 11:06, Krishna Addepalli wrote: The fix I propose is modeled after the code in Windows, which tracks items by their selected index. The code in Mac is storing currently selected object, which could raise the question about which object to select next, when the selected object is removed/changed. With index based approach, I see that, the index remains unchanged, even if the object at that location is removed/changed, unless it causes the index to be out of bounds. This behavior is the same across Windows, Linux and Mac. I guess behavior is the same, because of the shared code in Choice->remove->removeNoInvalidate which selects the new element if the old selected element was removed. And it is not affected by the code changed in the fix. -- Best regards, Sergey. -- Best regards, Sergey.
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Hi Sergey, I had to update the test, since on Mac, it is always selecting the second element, and the test fails. Thanks, Krishna -Original Message- From: Sergey Bylokhov Sent: Monday, October 1, 2018 7:57 AM To: Krishna Addepalli Cc: Ajit Ghaisas ; Shashidhara Veerabhadraiah ; awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi, Krishna. Why did you update the test? Before the fix the robot used the middle of the first element, but after the fix it will click on the first pixel of the first element(it can missclick on the choice itself), not sure it will be stable enough. On 21/09/2018 15:30, Krishna Addepalli wrote: > Hi Sergey, > > I have not been able to find any example of different behaviour > between index based and object based logic, so I have made the changes > on Mac as you suggested. Also, on Linux, I found that the Choice.java > class holds a selectedIndex already, so I removed it in XChoicePeer.java, and > tested it. > Here is the new webrev: > http://cr.openjdk.java.net/~kaddepalli/8014503/webrev03/ > <http://cr.openjdk.java.net/%7Ekaddepalli/8014503/webrev03/> > > Thanks, > Krishna > >> On 23-Aug-2018, at 4:34 PM, Sergey Bylokhov >> mailto:sergey.bylok...@oracle.com>> wrote: >> >> Hi, Krishna. >> On 20/08/2018 04:30, Krishna Addepalli wrote: >>> Yes, the behavior is not because of the fix I made. I was trying to >>> understand the implications of the index based selection approach, >>> and now it looks to be consistent on all the platforms. >>> On Mac, as per your suggestion, we can get the same behavior by >>> removing the "setSelectedItem", but my question is, should I make >>> this change or keep the index based change, since that would keep >>> the implementation also consistent across platforms. >> >> Can you please provide an example which will show the difference >> between these two solutions. >> >> btw initially the code in setSelectedItem(..) was added to fix the >> opposite bug. And current fix will skips some events which are >> generated to fix the opposite bug, it looks strange. >> >>> Thanks, >>> Krishna >>> -Original Message- >>> From: Sergey Bylokhov >>> Sent: Tuesday, August 14, 2018 7:52 AM >>> To: Krishna Addepalli >> <mailto:krishna.addepa...@oracle.com>>; Ajit Ghaisas >>> mailto:ajit.ghai...@oracle.com>>; >>> Shashidhara Veerabhadraiah >> <mailto:shashidhara.veerabhadra...@oracle.com>>; >>> awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net> >>> Subject: Re: [12]RFR : JDK-8014503: AWT Choice >>> implementation should be made consistent across platforms. >>> On 13/08/2018 11:06, Krishna Addepalli wrote: >>>> The fix I propose is modeled after the code in Windows, which >>>> tracks items by their selected index. The code in Mac is storing >>>> currently selected object, which could raise the question about >>>> which object to select next, when the selected object is removed/changed. >>>> With index based approach, I see that, the index remains unchanged, >>>> even if the object at that location is removed/changed, unless it >>>> causes the index to be out of bounds. >>>> This behavior is the same across Windows, Linux and Mac. >>> I guess behavior is the same, because of the shared code in >>> Choice->remove->removeNoInvalidate >>> which selects the new element if the old selected element was removed. >>> And it is not affected by the code changed in the fix. >> >> >> -- >> Best regards, Sergey. > -- Best regards, Sergey.
Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.
Looks fine. On 04/10/2018 02:05, Krishna Addepalli wrote: Hi Sergey, I had to update the test, since on Mac, it is always selecting the second element, and the test fails. Thanks, Krishna -Original Message- From: Sergey Bylokhov Sent: Monday, October 1, 2018 7:57 AM To: Krishna Addepalli Cc: Ajit Ghaisas ; Shashidhara Veerabhadraiah ; awt-dev@openjdk.java.net Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. Hi, Krishna. Why did you update the test? Before the fix the robot used the middle of the first element, but after the fix it will click on the first pixel of the first element(it can missclick on the choice itself), not sure it will be stable enough. On 21/09/2018 15:30, Krishna Addepalli wrote: Hi Sergey, I have not been able to find any example of different behaviour between index based and object based logic, so I have made the changes on Mac as you suggested. Also, on Linux, I found that the Choice.java class holds a selectedIndex already, so I removed it in XChoicePeer.java, and tested it. Here is the new webrev: http://cr.openjdk.java.net/~kaddepalli/8014503/webrev03/ <http://cr.openjdk.java.net/%7Ekaddepalli/8014503/webrev03/> Thanks, Krishna On 23-Aug-2018, at 4:34 PM, Sergey Bylokhov mailto:sergey.bylok...@oracle.com>> wrote: Hi, Krishna. On 20/08/2018 04:30, Krishna Addepalli wrote: Yes, the behavior is not because of the fix I made. I was trying to understand the implications of the index based selection approach, and now it looks to be consistent on all the platforms. On Mac, as per your suggestion, we can get the same behavior by removing the "setSelectedItem", but my question is, should I make this change or keep the index based change, since that would keep the implementation also consistent across platforms. Can you please provide an example which will show the difference between these two solutions. btw initially the code in setSelectedItem(..) was added to fix the opposite bug. And current fix will skips some events which are generated to fix the opposite bug, it looks strange. Thanks, Krishna -Original Message- From: Sergey Bylokhov Sent: Tuesday, August 14, 2018 7:52 AM To: Krishna Addepalli mailto:krishna.addepa...@oracle.com>>; Ajit Ghaisas mailto:ajit.ghai...@oracle.com>>; Shashidhara Veerabhadraiah mailto:shashidhara.veerabhadra...@oracle.com>>; awt-dev@openjdk.java.net <mailto:awt-dev@openjdk.java.net> Subject: Re: [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms. On 13/08/2018 11:06, Krishna Addepalli wrote: The fix I propose is modeled after the code in Windows, which tracks items by their selected index. The code in Mac is storing currently selected object, which could raise the question about which object to select next, when the selected object is removed/changed. With index based approach, I see that, the index remains unchanged, even if the object at that location is removed/changed, unless it causes the index to be out of bounds. This behavior is the same across Windows, Linux and Mac. I guess behavior is the same, because of the shared code in Choice->remove->removeNoInvalidate which selects the new element if the old selected element was removed. And it is not affected by the code changed in the fix. -- Best regards, Sergey. -- Best regards, Sergey.
