<AWT Dev> [12]RFR : JDK-8014503: AWT Choice implementation should be made consistent across platforms.

Krishna Addepalli krishna.addepalli at oracle.com
Mon Aug 13 18:06:32 UTC 2018


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(10000);
            c.remove("Item 1");
            System.out.println(c.getSelectedIndex());

            sleep(10000);

            //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 <krishna.addepalli at oracle.com>; Ajit Ghaisas <ajit.ghaisas at oracle.com>; Shashidhara Veerabhadraiah <shashidhara.veerabhadraiah at oracle.com>; awt-dev at openjdk.java.net
Subject: Re: <AWT Dev> [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 <krishna.addepalli at oracle.com>; Shashidhara 
> Veerabhadraiah <shashidhara.veerabhadraiah at oracle.com>;
> awt-dev at openjdk.java.net
> *Subject:* RE: <AWT Dev> [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 at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
> *Subject:* RE: <AWT Dev> [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 
> <shashidhara.veerabhadraiah at oracle.com
> <mailto:shashidhara.veerabhadraiah at oracle.com>>; Krishna Addepalli 
> <krishna.addepalli at oracle.com <mailto:krishna.addepalli at oracle.com>>;
> awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
> *Subject:* RE: <AWT Dev> [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 at openjdk.java.net 
> <mailto:awt-dev at openjdk.java.net>
> *Subject:* Re: <AWT Dev> [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 
> <shashidhara.veerabhadraiah at oracle.com
> <mailto:shashidhara.veerabhadraiah at oracle.com>>;
> awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
> *Subject:* RE: <AWT Dev> [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 <krishna.addepalli at oracle.com 
> <mailto:krishna.addepalli at oracle.com>>; awt-dev at openjdk.java.net 
> <mailto:awt-dev at openjdk.java.net>
> *Subject:* RE: <AWT Dev> [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 at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
> *Subject:* <AWT Dev> [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
> 


--
Best regards, Sergey.


More information about the awt-dev mailing list