Re: [cp-patches] java.awt.Choice selection behaviour when when no peer is set

[Noa Resare]

sön 2004-10-24 klockan 10:31 +0200 skrev Mark Wielaard: 
> Hi,
> 
> On Sun, 2004-10-24 at 00:24, Noa Resare wrote:
> > If you just instantiate a java.awt.Choice object and add/remove items
> > the rules for what item gets selected isn't obeyed since there is no
> > peer widget set instantiated. The attached patch implements this
> > behavior, so that the mauve testcases work.
> 
> Thanks. Don't forget to update the copyright year in the files you edit.
> And in general we try to list all methods that changed explicitly like:
> 
> 2004-10-22  Noa Resare  <address@hidden>
> 
>        * java/awt/Choice.java (add):
>        Implement correct selection behavior when peer == null.
>        (insert): Likewise.
>        (remove): Likewise.
> 
> And the entries should be full sentences, ending with a dot. See also:
>   "Documenting what changed when with ChangeLog entries"
>   http://www.gnu.org/software/classpath/docs/hacking.html#SEC9

Ok. I'll try to stick to that in the future. Thanks for the friendly
correction. 

> > The code I have added only affects the no-peer case, so any errors in it
> > shouldn't effect anyone doing anything real :)
> 
> Which I actually think is a flaw in your patch.
> The tests in mauve were added since some programs first set up a Choice,
> fill it with items and/or manipulate it somehow before it gets shown.
> So I think something like the following (untested) needs to be added:
> 
>       * gnu/java/awt/peer/gtk/GtkChoicePeer.java (GtkChoicePeer):
>       Call select() when Choice has a selected item.

I have tested your code with the attached very simple case, and it seems
to work like expected. 

> > +      if (index == selectedIndex)
> > +       select(0);
> > +      if (getItemCount() == 0)
> > +       selectedIndex = -1;
> 
> I think this should be:
> 
>       if (getItemCount() == 0)
>        selectedIndex = -1;
>       else if (index == selectedIndex)
>        select(0);
> 
> Otherwise you will get a IllegalArgumentException from select(0) if you
> remove the last item.
> 
> What do you think?

Actually, you get away without an IllegalArgumentException as it seems
like select() implements a literal interpretation of the specification
("IllegalArgumentException - if the specified position is greater than
the number of items or less than zero") which is not taking into account
that position is 0-based and 'number of items' is 1-based, so it is
actually legal to place the position right after the last item.

Of course your solution is better.

cheers/noa

ChoiceTester.java
Description: Text Data