diff --git a/widget/public/nsIMenu.h b/widget/public/nsIMenu.h index bc6af85099cc..400771f5446d 100644 --- a/widget/public/nsIMenu.h +++ b/widget/public/nsIMenu.h @@ -41,7 +41,6 @@ #include "nsISupports.h" #include "nsStringFwd.h" -class nsIDocShell; class nsIMenuBar; class nsIMenu; class nsIMenuItem; @@ -68,8 +67,8 @@ class nsIMenu : public nsISupports { * Creates the Menu * */ - NS_IMETHOD Create ( nsISupports * aParent, const nsAString &aLabel, const nsAString &aAccessKey, - nsIChangeManager* aManager, nsIDocShell* aShell, nsIContent* aNode ) = 0; + NS_IMETHOD Create(nsISupports * aParent, const nsAString &aLabel, const nsAString &aAccessKey, + nsIChangeManager* aManager, nsIContent* aNode) = 0; /** * Get the Menu's Parent. This addrefs. @@ -120,13 +119,6 @@ class nsIMenu : public nsISupports { */ NS_IMETHOD AddItem(nsISupports* aItem) = 0; - /** - * 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 visible menu items * This includes separators. It does not include hidden items. diff --git a/widget/public/nsIMenuItem.h b/widget/public/nsIMenuItem.h index 6d86b68443d3..729ccac6a3ce 100644 --- a/widget/public/nsIMenuItem.h +++ b/widget/public/nsIMenuItem.h @@ -42,7 +42,6 @@ #include "nsISupports.h" #include "nsString.h" -#include "nsIDocShell.h" #include "nsIDOMElement.h" // {F9A30AA5-D526-4C19-8418-C21BF6B31837} @@ -72,16 +71,15 @@ class nsIMenuItem : public nsISupports { public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_IMENUITEM_IID) - enum EMenuItemType { eRegular = 0, eCheckbox, eRadio } ; + enum EMenuItemType { eRegular = 0, eCheckbox, eRadio, eSeparator} ; /** * Creates the MenuItem * */ - NS_IMETHOD Create(nsIMenu* aParent, const nsString & aLabel, PRBool isSeparator, - EMenuItemType aItemType, nsIChangeManager* aManager, - nsIDocShell* aShell, nsIContent* aNode) = 0; - + NS_IMETHOD Create(nsIMenu* aParent, const nsString & aLabel, EMenuItemType aItemType, + nsIChangeManager* aManager, nsIContent* aNode) = 0; + /** * Get the MenuItem label * diff --git a/widget/src/cocoa/nsMenuBarX.mm b/widget/src/cocoa/nsMenuBarX.mm index 15e341b937b0..71ef1dcc8b03 100644 --- a/widget/src/cocoa/nsMenuBarX.mm +++ b/widget/src/cocoa/nsMenuBarX.mm @@ -445,14 +445,13 @@ nsMenuBarX::MenuConstruct(const nsMenuEvent & aMenuEvent, nsIWidget* aParentWind menu->GetAttr(kNameSpaceID_None, nsWidgetAtoms::accesskey, menuAccessKey); // Don't create the whole menu yet, just add in the top level names - + // Create nsMenu, the menubar will own it nsCOMPtr pnsMenu(do_CreateInstance(kMenuCID)); if (pnsMenu) { pnsMenu->Create(static_cast(this), menuName, menuAccessKey, - static_cast(this), - nsnull, menu); - + static_cast(this), menu); + // Make nsMenu a child of nsMenuBar. nsMenuBar takes ownership. AddMenu(pnsMenu); diff --git a/widget/src/cocoa/nsMenuItemIconX.mm b/widget/src/cocoa/nsMenuItemIconX.mm index dc5ed033c042..96e30e14469e 100644 --- a/widget/src/cocoa/nsMenuItemIconX.mm +++ b/widget/src/cocoa/nsMenuItemIconX.mm @@ -154,8 +154,7 @@ nsMenuItemIconX::GetIconURI(nsIURI** aIconURI) if (menuItem) { nsIMenuItem::EMenuItemType menuItemType; menuItem->GetMenuItemType(&menuItemType); - if (menuItemType == nsIMenuItem::eCheckbox || - menuItemType == nsIMenuItem::eRadio) + if (menuItemType != nsIMenuItem::eRegular) return NS_ERROR_FAILURE; } diff --git a/widget/src/cocoa/nsMenuItemX.h b/widget/src/cocoa/nsMenuItemX.h index e26483037aa0..ff83676c37b0 100644 --- a/widget/src/cocoa/nsMenuItemX.h +++ b/widget/src/cocoa/nsMenuItemX.h @@ -71,9 +71,8 @@ public: NS_DECL_NSICHANGEOBSERVER // nsIMenuItem Methods - NS_IMETHOD Create(nsIMenu* aParent, const nsString & aLabel, PRBool aIsSeparator, - EMenuItemType aItemType, nsIChangeManager* aManager, - nsIDocShell* aShell, nsIContent* aNode); + NS_IMETHOD Create(nsIMenu* aParent, const nsString & aLabel, EMenuItemType aItemType, + nsIChangeManager* aManager, nsIContent* aNode); NS_IMETHOD GetLabel(nsString &aText); NS_IMETHOD SetShortcutChar(const nsString &aText); NS_IMETHOD GetShortcutChar(nsString &aText); @@ -122,10 +121,9 @@ protected: nsRefPtr mIcon; PRUint8 mModifiers; - PRPackedBool mIsSeparator; PRPackedBool mEnabled; PRPackedBool mIsChecked; - EMenuItemType mMenuType; + EMenuItemType mType; // regular, checkbox, radio, or separator }; #endif // nsMenuItemX_h_ diff --git a/widget/src/cocoa/nsMenuItemX.mm b/widget/src/cocoa/nsMenuItemX.mm index 1a48dc59efb2..1ca20ee68a4d 100644 --- a/widget/src/cocoa/nsMenuItemX.mm +++ b/widget/src/cocoa/nsMenuItemX.mm @@ -67,11 +67,10 @@ nsMenuItemX::nsMenuItemX() mNativeMenuItem = nil; mMenuParent = nsnull; mManager = nsnull; - mIsSeparator = PR_FALSE; mKeyEquivalent.AssignLiteral(" "); mEnabled = PR_TRUE; mIsChecked = PR_FALSE; - mMenuType = eRegular; + mType = eRegular; } @@ -85,15 +84,14 @@ nsMenuItemX::~nsMenuItemX() } -NS_METHOD nsMenuItemX::Create(nsIMenu* aParent, const nsString & aLabel, PRBool aIsSeparator, - EMenuItemType aItemType, nsIChangeManager* aManager, - nsIDocShell* aShell, nsIContent* aNode) +NS_METHOD nsMenuItemX::Create(nsIMenu* aParent, const nsString & aLabel, EMenuItemType aItemType, + nsIChangeManager* aManager, nsIContent* aNode) { mContent = aNode; // addref mMenuParent = aParent; // weak - - mMenuType = aItemType; - + + mType = aItemType; + // register for AttributeChanged messages mManager = aManager; nsCOMPtr obs = do_QueryInterface(static_cast(this)); @@ -126,11 +124,10 @@ NS_METHOD nsMenuItemX::Create(nsIMenu* aParent, const nsString & aLabel, PRBool else mEnabled = !mContent->AttrValueIs(kNameSpaceID_None, nsWidgetAtoms::disabled, nsWidgetAtoms::_true, eCaseMatters); - mIsSeparator = aIsSeparator; mLabel = aLabel; // set up the native menu item - if (aIsSeparator) { + if (mType == nsIMenuItem::eSeparator) { mNativeMenuItem = [[NSMenuItem separatorItem] retain]; } else { @@ -218,7 +215,7 @@ NS_METHOD nsMenuItemX::GetChecked(PRBool *aIsEnabled) NS_METHOD nsMenuItemX::GetMenuItemType(EMenuItemType *aType) { - *aType = mMenuType; + *aType = mType; return NS_OK; } @@ -247,7 +244,7 @@ NS_METHOD nsMenuItemX::RemoveMenuListener(nsIMenuListener * aMenuListener) NS_METHOD nsMenuItemX::IsSeparator(PRBool & aIsSep) { - aIsSep = mIsSeparator; + aIsSep = (mType == nsIMenuItem::eSeparator); return NS_OK; } @@ -312,11 +309,11 @@ nsEventStatus nsMenuItemX::SetRebuild(PRBool aNeedsRebuild) // Executes the "cached" javaScript command. // Returns NS_OK if the command was executed properly, otherwise an error code. -NS_METHOD nsMenuItemX::DoCommand() +NS_IMETHODIMP nsMenuItemX::DoCommand() { // flip "checked" state if we're a checkbox menu, or an un-checked radio menu - if (mMenuType == nsIMenuItem::eCheckbox || - (mMenuType == nsIMenuItem::eRadio && !mIsChecked)) { + if (mType == nsIMenuItem::eCheckbox || + (mType == nsIMenuItem::eRadio && !mIsChecked)) { if (!mContent->AttrValueIs(kNameSpaceID_None, nsWidgetAtoms::autocheck, nsWidgetAtoms::_false, eCaseMatters)) SetChecked(!mIsChecked); @@ -327,7 +324,7 @@ NS_METHOD nsMenuItemX::DoCommand() nsXULCommandEvent event(PR_TRUE, NS_XUL_COMMAND, nsnull); mContent->DispatchDOMEvent(&event, nsnull, nsnull, &status); - return nsEventStatus_eConsumeNoDefault; + return NS_OK; } @@ -460,7 +457,7 @@ nsMenuItemX::AttributeChanged(nsIDocument *aDocument, PRInt32 aNameSpaceID, nsIC if (aAttribute == nsWidgetAtoms::checked) { // if we're a radio menu, uncheck our sibling radio items. No need to // do any of this if we're just a normal check menu. - if (mMenuType == eRadio) { + if (mType == nsIMenuItem::eRadio) { if (mContent->AttrValueIs(kNameSpaceID_None, nsWidgetAtoms::checked, nsWidgetAtoms::_true, eCaseMatters)) UncheckRadioSiblings(mContent); diff --git a/widget/src/cocoa/nsMenuX.h b/widget/src/cocoa/nsMenuX.h index c79f2c66b377..b4cd55c89057 100644 --- a/widget/src/cocoa/nsMenuX.h +++ b/widget/src/cocoa/nsMenuX.h @@ -96,15 +96,14 @@ public: nsEventStatus SetRebuild(PRBool aMenuEvent); // nsIMenu Methods - NS_IMETHOD Create (nsISupports * aParent, const nsAString &aLabel, const nsAString &aAccessKey, - nsIChangeManager* aManager, nsIDocShell* aShell, nsIContent* aNode); + NS_IMETHOD Create(nsISupports * aParent, const nsAString &aLabel, const nsAString &aAccessKey, + nsIChangeManager* aManager, nsIContent* aNode); NS_IMETHOD GetParent(nsISupports *&aParent); NS_IMETHOD GetLabel(nsString &aText); NS_IMETHOD SetLabel(const nsAString &aText); NS_IMETHOD GetAccessKey(nsString &aText); NS_IMETHOD SetAccessKey(const nsAString &aText); NS_IMETHOD AddItem(nsISupports* aText); - NS_IMETHOD AddSeparator(); NS_IMETHOD GetItemCount(PRUint32 &aCount); NS_IMETHOD GetItemAt(const PRUint32 aPos, nsISupports *& aMenuItem); NS_IMETHOD GetVisibleItemCount(PRUint32 &aCount); @@ -145,7 +144,6 @@ protected: void LoadMenuItem(nsIContent* inMenuItemContent); void LoadSubMenu(nsIContent* inMenuContent); - void LoadSeparator(nsIContent* inSeparatorContent); NSMenu* CreateMenuWithGeckoString(nsString& menuTitle); diff --git a/widget/src/cocoa/nsMenuX.mm b/widget/src/cocoa/nsMenuX.mm index 3d7fd0b0dafd..1e5162d4f5a3 100644 --- a/widget/src/cocoa/nsMenuX.mm +++ b/widget/src/cocoa/nsMenuX.mm @@ -86,22 +86,6 @@ static PRBool gConstructingMenu = PR_FALSE; static NS_DEFINE_CID(kMenuCID, NS_MENU_CID); static NS_DEFINE_CID(kMenuItemCID, NS_MENUITEM_CID); -// Refcounted class for dummy menu items like separators -class nsDummyMenuItemX : public nsISupports { -public: - NS_DECL_ISUPPORTS - - nsDummyMenuItemX() - { - } -}; - -NS_IMETHODIMP_(nsrefcnt) nsDummyMenuItemX::AddRef() { return ++mRefCnt; } -NS_IMETHODIMP nsDummyMenuItemX::Release() { return --mRefCnt; } -NS_IMPL_QUERY_INTERFACE0(nsDummyMenuItemX) -static nsDummyMenuItemX gDummyMenuItemX; - - NS_IMPL_ISUPPORTS4(nsMenuX, nsIMenu, nsIMenuListener, nsIChangeObserver, nsISupportsWeakReference) @@ -133,7 +117,7 @@ nsMenuX::~nsMenuX() NS_IMETHODIMP nsMenuX::Create(nsISupports * aParent, const nsAString &aLabel, const nsAString &aAccessKey, - nsIChangeManager* aManager, nsIDocShell* aShell, nsIContent* aNode) + nsIChangeManager* aManager, nsIContent* aNode) { mMenuContent = aNode; @@ -308,17 +292,6 @@ nsresult nsMenuX::AddMenu(nsIMenu * aMenu) } -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 - ++mVisibleItemsCount; - [mMacMenu addItem:[NSMenuItem separatorItem]]; - return NS_OK; -} - - // Includes all items, including hidden/collapsed ones NS_IMETHODIMP nsMenuX::GetItemCount(PRUint32 &aCount) { @@ -346,10 +319,7 @@ static PRBool MenuNodeIsVisible(nsISupports *item) // Find the content for this item in the menu, be it a MenuItem or a Menu nsCOMPtr itemContent; nsCOMPtr menuItem = do_QueryInterface(item); - if (item == &gDummyMenuItemX) { - return PR_TRUE; - } - else if (menuItem) { + if (menuItem) { menuItem->GetMenuItemContent(getter_AddRefs(itemContent)); } else { @@ -596,10 +566,8 @@ nsEventStatus nsMenuX::MenuConstruct( if (child) { // depending on the type, create a menu item, separator, or submenu nsIAtom *tag = child->Tag(); - if (tag == nsWidgetAtoms::menuitem) + if (tag == nsWidgetAtoms::menuitem || tag == nsWidgetAtoms::menuseparator) LoadMenuItem(child); - else if (tag == nsWidgetAtoms::menuseparator) - LoadSeparator(child); else if (tag == nsWidgetAtoms::menu) LoadSubMenu(child); } @@ -707,18 +675,22 @@ void nsMenuX::LoadMenuItem(nsIContent* inMenuItemContent) // printf("menuitem %s \n", NS_LossyConvertUTF16toASCII(menuitemName).get()); - static nsIContent::AttrValuesArray strings[] = - {&nsWidgetAtoms::checkbox, &nsWidgetAtoms::radio, nsnull}; nsIMenuItem::EMenuItemType itemType = nsIMenuItem::eRegular; - switch (inMenuItemContent->FindAttrValueIn(kNameSpaceID_None, nsWidgetAtoms::type, - strings, eCaseMatters)) { - case 0: itemType = nsIMenuItem::eCheckbox; break; - case 1: itemType = nsIMenuItem::eRadio; break; + if (inMenuItemContent->Tag() == nsWidgetAtoms::menuseparator) { + itemType = nsIMenuItem::eSeparator; + } + else { + static nsIContent::AttrValuesArray strings[] = + {&nsWidgetAtoms::checkbox, &nsWidgetAtoms::radio, nsnull}; + switch (inMenuItemContent->FindAttrValueIn(kNameSpaceID_None, nsWidgetAtoms::type, + strings, eCaseMatters)) { + case 0: itemType = nsIMenuItem::eCheckbox; break; + case 1: itemType = nsIMenuItem::eRadio; break; + } } // Create the item. - pnsMenuItem->Create(this, menuitemName, PR_FALSE, itemType, mManager, - nsnull, inMenuItemContent); + pnsMenuItem->Create(this, menuitemName, itemType, mManager, inMenuItemContent); AddMenuItem(pnsMenuItem); @@ -739,7 +711,7 @@ void nsMenuX::LoadSubMenu(nsIContent* inMenuContent) if (!pnsMenu) return; - pnsMenu->Create(reinterpret_cast(this), menuName, EmptyString(), mManager, nsnull, inMenuContent); + pnsMenu->Create(reinterpret_cast(this), menuName, EmptyString(), mManager, inMenuContent); AddMenu(pnsMenu); @@ -749,17 +721,6 @@ void nsMenuX::LoadSubMenu(nsIContent* inMenuContent) } -void nsMenuX::LoadSeparator(nsIContent* inSeparatorContent) -{ - // See bug 375011. - // Currently we don't create nsIMenuItem objects for separators so we can't - // track changes in their hidden/collapsed attributes. If it is hidden now it - // is hidden forever. - if (!NodeIsHiddenOrCollapsed(inSeparatorContent)) - AddSeparator(); -} - - // Fire our oncreate handler. Returns TRUE if we should keep processing the event, // FALSE if the handler wants to stop the creation of the menu PRBool nsMenuX::OnCreate()