XP menu changes. Full keyboard navigation now works. Next up: cascading.

layout/xul/base/public/nsIMenuParent.h

@@ -34,6 +34,8 @@ public:
   NS_IMETHOD SetCurrentMenuItem(nsIFrame* aMenuItem) = 0;
   NS_IMETHOD GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult) = 0;
   NS_IMETHOD GetPreviousMenuItem(nsIFrame* aStart, nsIFrame** aResult) = 0;
+
+  NS_IMETHOD SetActive() = 0;
 };
 
 #endif

layout/xul/base/src/nsIMenuParent.h

@@ -34,6 +34,8 @@ public:
   NS_IMETHOD SetCurrentMenuItem(nsIFrame* aMenuItem) = 0;
   NS_IMETHOD GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult) = 0;
   NS_IMETHOD GetPreviousMenuItem(nsIFrame* aStart, nsIFrame** aResult) = 0;
+
+  NS_IMETHOD SetActive() = 0;
 };
 
 #endif

layout/xul/base/src/nsMenuBarFrame.cpp

@@ -29,14 +29,7 @@
 #include "nsIDocument.h"
 #include "nsIDOMEventReceiver.h"
 #include "nsXULAtoms.h"
-
-// XXX Notes to self
-// 1. The menu bar should get its own view, and it should capture all events when the ALT key
-// goes down.
-//
-// 2. Collapsing the menu bar will destroy the frame. The menu bar will need to unregister its
-// listeners when this happens.
-//
+#include "nsMenuFrame.h"
 
 //
 // NS_NewMenuBarFrame
@@ -128,9 +121,9 @@ nsMenuBarFrame::ToggleMenuActiveState()
     mIsActive = PR_FALSE;
     if (mCurrentMenu) {
       // Deactivate the menu.
-      nsCOMPtr<nsIContent> content;
-      mCurrentMenu->GetContent(getter_AddRefs(content));
-      content->UnsetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, PR_TRUE);
+      nsMenuFrame* menuFrame = (nsMenuFrame*)mCurrentMenu;
+      menuFrame->OpenMenu(PR_FALSE);
+      menuFrame->SelectMenu(PR_FALSE);
       mCurrentMenu = nsnull;
     }
   }
@@ -144,10 +137,9 @@ nsMenuBarFrame::ToggleMenuActiveState()
     nsIFrame* firstFrame;
     GetNextMenuItem(nsnull, &firstFrame);
     if (firstFrame) {
-      // Activate the item.
-      firstFrame->GetContent(getter_AddRefs(firstMenuItem));
-      firstMenuItem->SetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, "true", PR_TRUE);
-
+      nsMenuFrame* menuFrame = (nsMenuFrame*)firstFrame;
+      menuFrame->SelectMenu(PR_TRUE);
+      
       // Track this item for keyboard navigation.
       mCurrentMenu = firstFrame;
     }
@@ -160,10 +152,19 @@ nsMenuBarFrame::KeyboardNavigation(PRUint32 aDirection)
   if (!mCurrentMenu)
     return;
 
-  if (aDirection == NS_VK_RIGHT ||
-      aDirection == NS_VK_LEFT) {
+  nsMenuFrame* menuFrame = (nsMenuFrame*)mCurrentMenu;
+   
+  PRBool handled = PR_FALSE;
+  if (menuFrame->IsOpen()) {
+    // Let the child menu try to handle it.
+    menuFrame->KeyboardNavigation(aDirection, handled);
+  }
+
+  if (handled)
+    return;
+
+  if (aDirection == NS_VK_RIGHT || aDirection == NS_VK_LEFT) {
     
-    // Activate the child at the position specified by index.
     nsIFrame* nextItem;
     
     if (aDirection == NS_VK_RIGHT)
@@ -172,13 +173,24 @@ nsMenuBarFrame::KeyboardNavigation(PRUint32 aDirection)
 
     SetCurrentMenuItem(nextItem);
   }
+  else if (aDirection == NS_VK_UP || aDirection == NS_VK_DOWN) {
+    // Open the menu and select its first item.
+    menuFrame->OpenMenu(PR_TRUE);
+    menuFrame->SelectFirstItem();
+  }
 }
 
 NS_IMETHODIMP
 nsMenuBarFrame::GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult)
 {
-  nsIFrame* currFrame = aStart ? aStart : mFrames.FirstChild();
-  currFrame->GetNextSibling(&currFrame);
+  nsIFrame* currFrame;
+  if (aStart) {
+    currFrame = aStart;
+    if (currFrame)
+      currFrame->GetNextSibling(&currFrame);
+  }
+  else currFrame = mFrames.FirstChild();
+
   while (currFrame) {
     nsCOMPtr<nsIContent> current;
     currFrame->GetContent(getter_AddRefs(current));
@@ -219,8 +231,14 @@ nsMenuBarFrame::GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult)
 NS_IMETHODIMP
 nsMenuBarFrame::GetPreviousMenuItem(nsIFrame* aStart, nsIFrame** aResult)
 {
-  nsIFrame* currFrame = aStart ? aStart : mFrames.LastChild();
-  currFrame = mFrames.GetPrevSiblingFor(currFrame);
+  nsIFrame* currFrame;
+  if (aStart) {
+    currFrame = aStart;
+    if (currFrame)
+      currFrame = mFrames.GetPrevSiblingFor(currFrame);
+  }
+  else currFrame = mFrames.LastChild();
+
   while (currFrame) {
     nsCOMPtr<nsIContent> current;
     currFrame->GetContent(getter_AddRefs(current));
@@ -263,19 +281,24 @@ NS_IMETHODIMP nsMenuBarFrame::SetCurrentMenuItem(nsIFrame* aMenuItem)
   if (mCurrentMenu == aMenuItem)
     return NS_OK;
 
- // Unset the current child.
+  nsMenuFrame* menuFrame = (nsMenuFrame*)mCurrentMenu;
+  PRBool wasOpen = PR_FALSE;
+  
+  // Unset the current child.
   if (mCurrentMenu) {
-    nsCOMPtr<nsIContent> content;
-    mCurrentMenu->GetContent(getter_AddRefs(content));
-    content->UnsetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, PR_TRUE);
+    wasOpen = menuFrame->IsOpen();
+    menuFrame->OpenMenu(PR_FALSE);
+    menuFrame->SelectMenu(PR_FALSE);
   }
 
   // Set the new child.
   if (aMenuItem) {
-    nsCOMPtr<nsIContent> content;
-    aMenuItem->GetContent(getter_AddRefs(content));
-    content->SetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, "true", PR_TRUE);
+    nsMenuFrame* newFrame = (nsMenuFrame*)aMenuItem;
+    newFrame->SelectMenu(PR_TRUE);
+    if (wasOpen)
+      newFrame->OpenMenu(PR_TRUE);
   }
+
   mCurrentMenu = aMenuItem;
 
   return NS_OK;

layout/xul/base/src/nsMenuBarFrame.h

@@ -45,7 +45,8 @@ public:
   NS_IMETHOD SetCurrentMenuItem(nsIFrame* aMenuItem);
   NS_IMETHOD GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult);
   NS_IMETHOD GetPreviousMenuItem(nsIFrame* aStart, nsIFrame** aResult);
-  
+  NS_IMETHOD SetActive() { mIsActive = PR_TRUE; return NS_OK; };
+
   NS_IMETHOD IsActive() { return mIsActive; };
 
   NS_IMETHOD Init(nsIPresContext&  aPresContext,

layout/xul/base/src/nsMenuFrame.cpp

@@ -246,8 +246,14 @@ nsMenuFrame::OpenMenu(PRBool aActivateFlag)
   if (aActivateFlag) {
     // Open the menu.
     mContent->SetAttribute(kNameSpaceID_None, nsXULAtoms::open, "true", PR_TRUE);
-    if (child)
+    if (child) {
+      // We've got some children for real.
       child->SetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, "true", PR_TRUE);
+      
+      // Tell the menu bar we're active.
+      mMenuParent->SetActive();
+    }
+
     mMenuOpen = PR_TRUE;
     //if (menuPopup)
     //  menuPopup->CaptureMouseEvents(PR_TRUE);
@@ -318,3 +324,18 @@ nsMenuFrame::Reflow(nsIPresContext&   aPresContext,
  
   return rv;
 }
+
+void
+nsMenuFrame::KeyboardNavigation(PRUint32 aDirection, PRBool& aHandledFlag)
+{
+  nsIFrame* frame = mPopupFrames.FirstChild();
+  if (frame) {
+    nsMenuPopupFrame* popup = (nsMenuPopupFrame*)frame;
+    popup->KeyboardNavigation(aDirection, aHandledFlag);
+  }
+}
+
+void
+nsMenuFrame::SelectFirstItem()
+{
+}

layout/xul/base/src/nsMenuFrame.h

@@ -77,10 +77,16 @@ public:
                     const nsHTMLReflowState& aReflowState,
                     nsReflowStatus&          aStatus);
 
+  void KeyboardNavigation(PRUint32 aDirection, PRBool& aHandledFlag);
+
   void ToggleMenuState();
   void SelectMenu(PRBool aActivateFlag);
   void OpenMenu(PRBool aActivateFlag);
 
+  void SelectFirstItem();
+
+  PRBool IsOpen() { return mMenuOpen; };
+
 protected:
   void GetMenuChildrenElement(nsIContent** aResult);
 

layout/xul/base/src/nsMenuPopupFrame.cpp

@@ -29,6 +29,7 @@
 #include "nsINameSpaceManager.h"
 #include "nsIViewManager.h"
 #include "nsWidgetsCID.h"
+#include "nsMenuFrame.h"
 
 const PRInt32 kMaxZ = 0x7fffffff; //XXX: Shouldn't there be a define somewhere for MaxInt for PRInt32
 
@@ -219,8 +220,14 @@ nsMenuPopupFrame::DidReflow(nsIPresContext& aPresContext,
 NS_IMETHODIMP
 nsMenuPopupFrame::GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult)
 {
-  nsIFrame* currFrame = aStart ? aStart : mFrames.FirstChild();
-  currFrame->GetNextSibling(&currFrame);
+  nsIFrame* currFrame;
+  if (aStart) {
+    currFrame = aStart; 
+    if (currFrame)
+      currFrame->GetNextSibling(&currFrame);
+  }
+  else currFrame = mFrames.FirstChild();
+  
   while (currFrame) {
     nsCOMPtr<nsIContent> current;
     currFrame->GetContent(getter_AddRefs(current));
@@ -261,8 +268,14 @@ nsMenuPopupFrame::GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult)
 NS_IMETHODIMP
 nsMenuPopupFrame::GetPreviousMenuItem(nsIFrame* aStart, nsIFrame** aResult)
 {
-  nsIFrame* currFrame = aStart ? aStart : mFrames.LastChild();
-  currFrame = mFrames.GetPrevSiblingFor(currFrame);
+  nsIFrame* currFrame;
+  if (aStart) {
+    currFrame = aStart;
+    if (currFrame)
+      currFrame = mFrames.GetPrevSiblingFor(currFrame);
+  }
+  else currFrame = mFrames.LastChild();
+
   while (currFrame) {
     nsCOMPtr<nsIContent> current;
     currFrame->GetContent(getter_AddRefs(current));
@@ -305,19 +318,24 @@ NS_IMETHODIMP nsMenuPopupFrame::SetCurrentMenuItem(nsIFrame* aMenuItem)
   if (mCurrentMenu == aMenuItem)
     return NS_OK;
 
- // Unset the current child.
+  nsMenuFrame* menuFrame = (nsMenuFrame*)mCurrentMenu;
+  PRBool wasOpen = PR_FALSE;
+  
+  // Unset the current child.
   if (mCurrentMenu) {
-    nsCOMPtr<nsIContent> content;
-    mCurrentMenu->GetContent(getter_AddRefs(content));
-    content->UnsetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, PR_TRUE);
+    wasOpen = menuFrame->IsOpen();
+    menuFrame->OpenMenu(PR_FALSE);
+    menuFrame->SelectMenu(PR_FALSE);
   }
 
   // Set the new child.
   if (aMenuItem) {
-    nsCOMPtr<nsIContent> content;
-    aMenuItem->GetContent(getter_AddRefs(content));
-    content->SetAttribute(kNameSpaceID_None, nsXULAtoms::menuactive, "true", PR_TRUE);
+    nsMenuFrame* newFrame = (nsMenuFrame*)aMenuItem;
+    newFrame->SelectMenu(PR_TRUE);
+    if (wasOpen)
+      newFrame->OpenMenu(PR_TRUE);
   }
+
   mCurrentMenu = aMenuItem;
 
   return NS_OK;
@@ -348,3 +366,38 @@ nsMenuPopupFrame::CaptureMouseEvents(PRBool aGrabMouseEvents)
   return NS_OK;
 }
 
+void
+nsMenuPopupFrame::KeyboardNavigation(PRUint32 aDirection, PRBool& aHandledFlag)
+{
+  // This method only gets called if we're open.
+  if (!mCurrentMenu) {
+    // We've been opened, but we haven't had anything selected.
+    // We can't handle LEFT or RIGHT, but our parent can.
+    if (aDirection == NS_VK_RIGHT || aDirection == NS_VK_LEFT)
+      return;
+  }
+
+  nsMenuFrame* menuFrame = (nsMenuFrame*)mCurrentMenu;
+  
+  if (menuFrame && menuFrame->IsOpen()) {
+    // Give our child a shot.
+    menuFrame->KeyboardNavigation(aDirection, aHandledFlag);
+  }
+
+  if (aHandledFlag)
+    return; // The child menu took it for us.
+
+  // For the vertical direction, we can move up or down.
+  if (aDirection == NS_VK_UP || aDirection == NS_VK_DOWN) {
+    
+    nsIFrame* nextItem;
+    
+    if (aDirection == NS_VK_DOWN)
+      GetNextMenuItem(mCurrentMenu, &nextItem);
+    else GetPreviousMenuItem(mCurrentMenu, &nextItem);
+
+    SetCurrentMenuItem(nextItem);
+
+    aHandledFlag = PR_TRUE;
+  }
+}

layout/xul/base/src/nsMenuPopupFrame.h

@@ -46,6 +46,7 @@ public:
   NS_IMETHOD SetCurrentMenuItem(nsIFrame* aMenuItem);
   NS_IMETHOD GetNextMenuItem(nsIFrame* aStart, nsIFrame** aResult);
   NS_IMETHOD GetPreviousMenuItem(nsIFrame* aStart, nsIFrame** aResult);
+  NS_IMETHOD SetActive() { return NS_OK; }; // We don't care.
 
   // Overridden methods
   NS_IMETHOD Init(nsIPresContext&  aPresContext,
@@ -62,6 +63,8 @@ public:
 
   NS_IMETHOD CaptureMouseEvents(PRBool aGrabMouseEvents);
 
+  void KeyboardNavigation(PRUint32 aDirection, PRBool& aHandledFlag);
+
 protected:
   nsIFrame* mCurrentMenu; // The current menu that is active.
   PRBool mIsCapturingMouseEvents; // Whether or not we're grabbing the mouse events.