allow DOM to unhide menus and menu items whether or not their initial state is hidden. fixes bidi menu items. b=364994 r=mano sr=roc

widget/public/nsIMenu.h

@@ -115,25 +115,29 @@ class nsIMenu : public nsISupports {
 	NS_IMETHOD GetEnabled(PRBool* aIsEnabled) = 0;
 	
 	/**
-    * Adds a Menu Item
+    * Adds a Menu Item. Do not use outside of widget menu implementations.
+    * Add and modify menu items via DOM content.
     *
     */
     NS_IMETHOD AddItem(nsISupports* aItem) = 0;
 
    /**
-    * Adds a separator
+    * Adds a separator. Do not use outside of widget menu implementations.
+    * Add and modify menu separators via DOM content.
     *
     */
     NS_IMETHOD AddSeparator() = 0;
 
    /**
     * Returns the number of menu items
-    * This does count separators as items
+    * This includes separators. It does not include hidden items.
+    *
     */
     NS_IMETHOD GetItemCount(PRUint32 &aCount) = 0;
 
    /**
-    * Returns a Menu or Menu Item at a specified Index
+    * Returns a Menu or Menu Item at a specified Index.
+    * This includes separators. It does not include hidden items.
     *
     */
     NS_IMETHOD GetItemAt(const PRUint32 aPos, nsISupports *& aMenuItem) = 0;

widget/public/nsIMenuItem.h

@@ -45,10 +45,10 @@
 #include "nsIDocShell.h"
 #include "nsIDOMElement.h"
 
-// {4E3931A7-D7E5-47FC-9489-83928536DA9D}
+// {F9A30AA5-D526-4C19-8418-C21BF6B31837}
 #define NS_IMENUITEM_IID \
-{ 0x4E3931A7, 0xD7E5, 0x47FC, \
+{ 0xF9A30AA5, 0xD526, 0x4C19, \
-  { 0x94, 0x89, 0x83, 0x92, 0x85, 0x36, 0xDA, 0x9D } }
+  { 0x84, 0x18, 0xC2, 0x1B, 0xF6, 0xB3, 0x18, 0x37 } }
 
 class nsIMenu;
 class nsIWidget;
@@ -170,6 +170,12 @@ class nsIMenuItem : public nsISupports {
     * Sets an appropriate icon for the menu item.
     */
     NS_IMETHOD SetupIcon() = 0;
+
+    /**
+     * Get GetMenuItemContent
+     *
+     */
+    NS_IMETHOD GetMenuItemContent(nsIContent ** aMenuItemContent) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIMenuItem, NS_IMENUITEM_IID)

widget/src/cocoa/nsMenuItemX.h

@@ -88,6 +88,7 @@ public:
   NS_IMETHOD SetModifiers(PRUint8 aModifiers);
   NS_IMETHOD GetModifiers(PRUint8 * aModifiers);
   NS_IMETHOD SetupIcon();
+  NS_IMETHOD GetMenuItemContent(nsIContent ** aMenuItemContent);
     
   // nsIMenuListener interface
   nsEventStatus MenuItemSelected(const nsMenuEvent & aMenuEvent);

widget/src/cocoa/nsMenuItemX.mm

@@ -513,3 +513,12 @@ nsMenuItemX::SetupIcon()
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
+
+
+NS_IMETHODIMP
+nsMenuItemX::GetMenuItemContent(nsIContent ** aMenuItemContent)
+{
+  NS_ENSURE_ARG_POINTER(aMenuItemContent);
+  NS_IF_ADDREF(*aMenuItemContent = mContent);
+  return NS_OK;
+}

widget/src/cocoa/nsMenuX.h

@@ -136,8 +136,8 @@ protected:
     nsresult AddMenuItem(nsIMenuItem * aMenuItem);
     nsresult AddMenu(nsIMenu * aMenu);
 
-    void LoadMenuItem(nsIMenu* pParentMenu, nsIContent* inMenuItemContent);  
+    void LoadMenuItem(nsIContent* inMenuItemContent);  
-    void LoadSubMenu(nsIMenu * pParentMenu, nsIContent* inMenuContent);
+    void LoadSubMenu(nsIContent* inMenuContent);
     void LoadSeparator(nsIContent* inSeparatorContent);
 
     NSMenu* CreateMenuWithGeckoString(nsString& menuTitle);
@@ -145,6 +145,7 @@ protected:
 protected:
     nsString                    mLabel;
     nsCOMArray<nsISupports>     mMenuItemsArray;
+    nsCOMArray<nsISupports>     mHiddenMenuItemsArray;
 
     nsISupports*                mParent;                // weak, my parent owns me
     nsIChangeManager*           mManager;               // weak ref, it will outlive us [menubar]

widget/src/cocoa/nsMenuX.mm

@@ -207,21 +207,26 @@ NS_IMETHODIMP nsMenuX::SetAccessKey(const nsAString &aText)
 }
 
 
+// This should only be used internally by our menu implementation. In all other
+// cases menus and their items should be added and modified via the DOM.
 NS_IMETHODIMP nsMenuX::AddItem(nsISupports* aItem)
 {
   nsresult rv = NS_ERROR_FAILURE;
-  if (aItem) {
+
-    // Figure out what we're adding
+  if (!aItem)
-    nsCOMPtr<nsIMenuItem> menuItem(do_QueryInterface(aItem));
+    return NS_ERROR_INVALID_ARG;
-    if (menuItem) {
+
-      rv = AddMenuItem(menuItem);
+  // Figure out what we're adding
-    }
+  nsCOMPtr<nsIMenuItem> menuItem(do_QueryInterface(aItem, &rv));
-    else {
+  if (NS_SUCCEEDED(rv)) {
-      nsCOMPtr<nsIMenu> menu(do_QueryInterface(aItem));
+    rv = AddMenuItem(menuItem);
-      if (menu)
-        rv = AddMenu(menu);
-    }
   }
+  else {
+    nsCOMPtr<nsIMenu> menu(do_QueryInterface(aItem, &rv));
+    if (NS_SUCCEEDED(rv))
+      rv = AddMenu(menu);
+  }
+
   return rv;
 }
 
@@ -231,7 +236,15 @@ nsresult nsMenuX::AddMenuItem(nsIMenuItem * aMenuItem)
   if (!aMenuItem)
     return NS_ERROR_INVALID_ARG;
 
-  mMenuItemsArray.AppendObject(aMenuItem); // owning ref
+  nsCOMPtr<nsIContent> menuItemContent;
+  aMenuItem->GetMenuItemContent(getter_AddRefs(menuItemContent));
+  if (menuItemContent && NodeIsHiddenOrCollapsed(menuItemContent)) {
+    mHiddenMenuItemsArray.AppendObject(aMenuItem); // owning ref
+    return NS_OK;
+  }
+  else {
+    mMenuItemsArray.AppendObject(aMenuItem); // owning ref
+  }
 
   // add the menu item to this menu
   NSMenuItem* newNativeMenuItem;
@@ -263,12 +276,15 @@ nsresult nsMenuX::AddMenu(nsIMenu * aMenu)
   if (!aMenu)
     return NS_ERROR_NULL_POINTER;
 
-  nsCOMPtr<nsISupports> supports = do_QueryInterface(aMenu);
+  nsCOMPtr<nsIContent> menuContent;
-  if (!supports)
+  aMenu->GetMenuContent(getter_AddRefs(menuContent));
-    return NS_ERROR_NO_INTERFACE;
+  if (menuContent && NodeIsHiddenOrCollapsed(menuContent)) {
-
+    mHiddenMenuItemsArray.AppendObject(aMenu); // owning ref
-  PRUint32 currItemIndex = mMenuItemsArray.Count();
+    return NS_OK;
-  mMenuItemsArray.AppendObject(supports); // owning ref
+  }
+  else {
+    mMenuItemsArray.AppendObject(aMenu); // owning ref
+  }
 
   // We have to add a menu item and then associate the menu with it
   nsAutoString label;
@@ -284,7 +300,7 @@ nsresult nsMenuX::AddMenu(nsIMenu * aMenu)
   
   NSMenu* childMenu;
   if (aMenu->GetNativeData((void**)&childMenu) == NS_OK)
-    [[mMacMenu itemAtIndex:currItemIndex] setSubmenu:childMenu];
+    [newNativeMenuItem setSubmenu:childMenu];
 
   return NS_OK;
 }
@@ -294,7 +310,7 @@ NS_IMETHODIMP nsMenuX::AddSeparator()
 {
   // We're not really appending an nsMenuItem but a placeholder needs to be
   // here to make sure that event dispatching isn't off by one.
-  mMenuItemsArray.AppendObject(&gDummyMenuItemX); // owning ref
+  mMenuItemsArray.AppendObject(&gDummyMenuItemX);  // owning ref
   [mMacMenu addItem:[NSMenuItem separatorItem]];
   return NS_OK;
 }
@@ -342,6 +358,7 @@ NS_IMETHODIMP nsMenuX::RemoveAll()
   }
   // get rid of Gecko menu items
   mMenuItemsArray.Clear();
+  mHiddenMenuItemsArray.Clear();
   
   return NS_OK;
 }
@@ -431,8 +448,7 @@ nsEventStatus nsMenuX::MenuSelected(const nsMenuEvent & aMenuEvent)
     // Make sure none of our submenus are the ones that should be handling this
     for (PRUint32 i = mMenuItemsArray.Count() - 1; i >= 0; i--) {
       nsISupports*              menuSupports = mMenuItemsArray.ObjectAt(i);
-      nsCOMPtr<nsIMenu>              submenu = do_QueryInterface(menuSupports);
+      nsCOMPtr<nsIMenuListener> menuListener = do_QueryInterface(menuSupports);
-      nsCOMPtr<nsIMenuListener> menuListener = do_QueryInterface(submenu);
       if (menuListener) {
         eventStatus = menuListener->MenuSelected(aMenuEvent);
         if (nsEventStatus_eIgnore != eventStatus)
@@ -511,11 +527,11 @@ nsEventStatus nsMenuX::MenuConstruct(
       // depending on the type, create a menu item, separator, or submenu
       nsIAtom *tag = child->Tag();
       if (tag == nsWidgetAtoms::menuitem)
-        LoadMenuItem(this, child);
+        LoadMenuItem(child);
       else if (tag == nsWidgetAtoms::menuseparator)
         LoadSeparator(child);
       else if (tag == nsWidgetAtoms::menu)
-        LoadSubMenu(this, child);
+        LoadSubMenu(child);
     }
   } // for each menu item
 
@@ -606,14 +622,11 @@ NSMenu* nsMenuX::CreateMenuWithGeckoString(nsString& menuTitle)
 }
 
 
-void nsMenuX::LoadMenuItem(nsIMenu* inParentMenu, nsIContent* inMenuItemContent)
+void nsMenuX::LoadMenuItem(nsIContent* inMenuItemContent)
 {
   if (!inMenuItemContent)
     return;
 
-  if (NodeIsHiddenOrCollapsed(inMenuItemContent))
-    return;
-
   // create nsMenuItem
   nsCOMPtr<nsIMenuItem> pnsMenuItem = do_CreateInstance(kMenuItemCID);
   if (!pnsMenuItem)
@@ -638,7 +651,7 @@ void nsMenuX::LoadMenuItem(nsIMenu* inParentMenu, nsIContent* inMenuItemContent)
     return;
 
   // Create the item.
-  pnsMenuItem->Create(inParentMenu, menuitemName, PR_FALSE, itemType, mManager,
+  pnsMenuItem->Create(this, menuitemName, PR_FALSE, itemType, mManager,
                       docShell, inMenuItemContent);
 
   // Set key shortcut and modifiers
@@ -677,16 +690,12 @@ void nsMenuX::LoadMenuItem(nsIMenu* inParentMenu, nsIContent* inMenuItemContent)
   else
     pnsMenuItem->SetChecked(PR_FALSE);
 
-  nsCOMPtr<nsISupports> supports(do_QueryInterface(pnsMenuItem));
+  AddMenuItem(pnsMenuItem);
-  inParentMenu->AddItem(supports); // Parent now owns menu item
 }
 
 
-void nsMenuX::LoadSubMenu(nsIMenu * pParentMenu, nsIContent* inMenuContent)
+void nsMenuX::LoadSubMenu(nsIContent* inMenuContent)
 {
-  if (NodeIsHiddenOrCollapsed(inMenuContent))
-    return;
-  
   nsAutoString menuName; 
   inMenuContent->GetAttr(kNameSpaceID_None, nsWidgetAtoms::label, menuName);
   //printf("Creating Menu [%s] \n", NS_LossyConvertUTF16toASCII(menuName).get());
@@ -700,8 +709,7 @@ void nsMenuX::LoadSubMenu(nsIMenu * pParentMenu, nsIContent* inMenuContent)
   nsCOMPtr<nsIDocShell> docShell = do_QueryReferent(mDocShellWeakRef);
   if (!docShell)
     return;
-  nsCOMPtr<nsISupports> supports(do_QueryInterface(pParentMenu));
+  pnsMenu->Create(NS_REINTERPRET_CAST(nsISupports*, this), menuName, EmptyString(), mManager, docShell, inMenuContent);
-  pnsMenu->Create(supports, menuName, EmptyString(), mManager, docShell, inMenuContent);
 
   // set if it's enabled or disabled
   if (inMenuContent->AttrValueIs(kNameSpaceID_None, nsWidgetAtoms::disabled,
@@ -710,18 +718,17 @@ void nsMenuX::LoadSubMenu(nsIMenu * pParentMenu, nsIContent* inMenuContent)
   else
     pnsMenu->SetEnabled(PR_TRUE);
 
-  // Make nsMenu a child of parent nsMenu. The parent takes ownership
+  AddMenu(pnsMenu);
-  nsCOMPtr<nsISupports> supports2(do_QueryInterface(pnsMenu));
-  pParentMenu->AddItem(supports2);
 }
 
 
 void nsMenuX::LoadSeparator(nsIContent* inSeparatorContent) 
 {
-  if (NodeIsHiddenOrCollapsed(inSeparatorContent))
+  // Currently we don't create nsIMenuItem objects for separators so we can't
-    return;
+  // track changes in their hidden/collapsed attributes. If it is hidden now it
-
+  // is hidden forever.
-  AddSeparator();
+  if (!NodeIsHiddenOrCollapsed(inSeparatorContent))
+    AddSeparator();
 }