[cp-patches] FYI: Menu.add/remove() item parent fixes

[Mark Wielaard]

Hi,

This is a similar patch as the one I made earlier for Frame and
MenuBars. I added some Mauve tests to make sure we always setup the
parent of a MenuComponent correctly when items get added or removed.

2006-02-14  Mark Wielaard  <[EMAIL PROTECTED]>

    * java/awt/Menu.java (add): Always set parent of item to this. Call
    addNotify() on item when we have a MenuPeer already.
    (insert): Always adjust parent for item. Call addNotify() on item if
    we already have a peer.
    (remove(int)): Always clear item parent. Call removeNotify() on item
    if we had a peer.

I also made some tests for MenuBar adding and removing Menus for Mauve,
but we seem to do the parent setting correctly in that case.

This makes my MenuMenu test that I posted earlier finally work. It does
give a couple of gtk+ warnings since we are sometimes trying to add
items twice (which is harmless, but not very nice). This is a bug in the
peers (both the MenuComponent and the MenuContainer try to add each
other). I'll try to fix that later.

Cheers,

Mark

Index: java/awt/Menu.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Menu.java,v
retrieving revision 1.21
diff -u -r1.21 Menu.java
--- java/awt/Menu.java	20 Sep 2005 01:05:28 -0000	1.21
+++ java/awt/Menu.java	14 Feb 2006 21:20:37 -0000
@@ -1,5 +1,5 @@
 /* Menu.java -- A Java AWT Menu
-   Copyright (C) 1999, 2002, 2004 Free Software Foundation, Inc.
+   Copyright (C) 1999, 2002, 2004, 2006 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -220,15 +220,15 @@
 public MenuItem
 add(MenuItem item)
 {
-  items.addElement(item);
   if (item.parent != null)
-    {
-      item.parent.remove(item);
-    }
-  item.parent = this;
+    item.parent.remove(item);
+
+  items.addElement(item);
+  item.setParent(this);
 
   if (peer != null)
     {
+      item.addNotify();
       MenuPeer mp = (MenuPeer) peer;
       mp.addItem(item);
     }
@@ -266,26 +266,32 @@
   if (index < 0)
     throw new IllegalArgumentException("Index is less than zero");
 
-  MenuPeer peer = (MenuPeer) getPeer();
-  if (peer == null)
-    return;
-
   int count = getItemCount ();
 
   if (index >= count)
-    peer.addItem (item);
+    add(item);
   else
     {
+      if (item.parent != null)
+	item.parent.remove(item);
+      
+      items.insertElementAt(item, index);
+      item.setParent(this);
+
+      MenuPeer peer = (MenuPeer) getPeer();
+      if (peer == null)
+	return;
+
       for (int i = count - 1; i >= index; i--)
-        peer.delItem (i);
+        peer.delItem(i);
 
-      peer.addItem (item);
+      item.addNotify();
+      peer.addItem(item);
 
       for (int i = index; i < count; i++)
-        peer.addItem ((MenuItem) items.elementAt (i));
+        peer.addItem((MenuItem) items.elementAt (i));
     }
 
-  items.insertElementAt(item, index);
 }
 
 /*************************************************************************/
@@ -344,11 +350,15 @@
 public synchronized void
 remove(int index)
 {
-  items.removeElementAt(index);
+  MenuItem item = (MenuItem) items.remove(index);
 
-  MenuPeer mp = (MenuPeer)getPeer();
+  MenuPeer mp = (MenuPeer) getPeer();
   if (mp != null)
-    mp.delItem(index);
+    {
+      mp.delItem(index);
+      item.removeNotify();
+    }
+  item.setParent(null);
 }
 
 /*************************************************************************/

signature.asc
Description: This is a digitally signed message part