Re: [PATCH] Fix for Choice

[Fernando Nasser <fnasser at redhat dot com>]

Tom Tromey wrote:
> > > > > > "Fernando" == Fernando Nasser <fnasser@redhat.com> writes:
>
>
> Fernando> The appearance is still not quite right.  Tom Fitzsimmons
> Fernando> and I even believe it should not be implemented using the
> Fernando> Gtk widgets it is currently using but at least this patch
> Fernando> makes it _behave_ like the one in the Sun SDK.
>
> Ok, thanks.


Thank you.

Just for the record, with the improved mauve ChoiceTest I found a couple of 
undocumented behaviors of the Sun AWT Choice.  It does generate an ItemEvent 
when the first element is added and does _not_ generate one when the last one is 
removed.  I do not necessarily agree with the design but I've adjusted our code 
to match.  The attached Choice.java file was the one actually checked in and is 
very slightly different from the one post (just the two changes above).

Index: java/awt/Choice.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/awt/Choice.java,v
retrieving revision 1.11
diff -c -p -r1.11 Choice.java
*** java/awt/Choice.java	5 Jun 2003 19:58:39 -0000	1.11
--- java/awt/Choice.java	23 Dec 2003 18:53:47 -0000
*************** add(String item)
*** 171,177 ****
      }
  
    if (i == 0)
!     select (0);
  }
  
  /*************************************************************************/
--- 171,185 ----
      }
  
    if (i == 0)
!   {
!     selectedIndex = 0;
!     // We must generate an ItemEvent here
!     Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent (
!       new ItemEvent ((ItemSelectable)this, 
!                      ItemEvent.ITEM_STATE_CHANGED,
!                      getItem(0),
!                      ItemEvent.SELECTED));
!   }
  }
  
  /*************************************************************************/
*************** insert(String item, int index)
*** 223,229 ****
--- 231,245 ----
      }
  
    if (getItemCount () == 1 || selectedIndex >= index)
+   {
      select (0);
+     // We must generate an ItemEvent here
+     Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent (
+       new ItemEvent ((ItemSelectable)this, 
+                      ItemEvent.ITEM_STATE_CHANGED,
+                      getItem(0),
+                      ItemEvent.SELECTED));
+   }
  }
  
  /*************************************************************************/
*************** remove(int index)
*** 265,272 ****
        cp.remove (index);
      }
  
!   if (index == selectedIndex)
      select (0);
    else if (selectedIndex > index)
      --selectedIndex;
  }
--- 281,296 ----
        cp.remove (index);
      }
  
!   if ((index == selectedIndex) && (getItemCount() > 0))
!   {
      select (0);
+     // We must generate an ItemEvent here
+     Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent (
+       new ItemEvent ((ItemSelectable)this, 
+                      ItemEvent.ITEM_STATE_CHANGED,
+                      getItem(0),
+                      ItemEvent.SELECTED));
+   }
    else if (selectedIndex > index)
      --selectedIndex;
  }
*************** removeAll()
*** 281,291 ****
  {
    int count = getItemCount();
  
!   for (int i = 0; i < count; i++)
      {
!       // Always remove 0.
!       remove(0);
      }
  }
  
  /*************************************************************************/
--- 305,331 ----
  {
    int count = getItemCount();
  
!   if (count <= 0)
!     return;
!   
!   ChoicePeer cp = (ChoicePeer) peer;
! 
!   // Select the first item to prevent an spurious ItemEvent to be generated
!   if (cp != null)
!     {
!       cp.select (0);
!       selectedIndex = 0; // Just to keep consistent
!     }
! 
!   for (int i = (count - 1); i >= 0; i--)
      {
!       // Always remove the last to avoid generation of ItemEvents.
!       pItems.removeElementAt(i);
!       if (cp != null)
!         cp.remove (i);
      }
+ 
+   selectedIndex = -1;
  }
  
  /*************************************************************************/