Bug 261715 Alt key auto-repeat shouldn't cause to show/focus menubar if mouse button is down during first Alt keydown and Alt keyup r=enn, a=jst
authorMasayuki Nakano <masayuki@d-toybox.com>
Sat, 04 Dec 2010 10:55:15 +0900
changeset 58591 70cb20ed0813dc2ab15652ae5887b8eef21d590f
parent 58590 e52f4987ec949b6e957baf517e85655e48413155
child 58592 e0285b393e11bf3a2fd7cb39538477af83e051ef
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersenn, jst
bugs261715
milestone2.0b8pre
Bug 261715 Alt key auto-repeat shouldn't cause to show/focus menubar if mouse button is down during first Alt keydown and Alt keyup r=enn, a=jst
layout/xul/base/src/nsMenuBarFrame.cpp
layout/xul/base/src/nsMenuBarListener.cpp
layout/xul/base/src/nsMenuBarListener.h
toolkit/content/tests/widgets/window_menubar.xul
--- a/layout/xul/base/src/nsMenuBarFrame.cpp
+++ b/layout/xul/base/src/nsMenuBarFrame.cpp
@@ -113,17 +113,19 @@ nsMenuBarFrame::Init(nsIContent*      aC
 
   // Also hook up the listener to the window listening for focus events. This is so we can keep proper
   // state as the user alt-tabs through processes.
   
   target->AddEventListener(NS_LITERAL_STRING("keypress"), (nsIDOMKeyListener*)mMenuBarListener, PR_FALSE); 
   target->AddEventListener(NS_LITERAL_STRING("keydown"), (nsIDOMKeyListener*)mMenuBarListener, PR_FALSE);  
   target->AddEventListener(NS_LITERAL_STRING("keyup"), (nsIDOMKeyListener*)mMenuBarListener, PR_FALSE);   
 
-  target->AddEventListener(NS_LITERAL_STRING("mousedown"), (nsIDOMMouseListener*)mMenuBarListener, PR_FALSE);   
+  // mousedown event should be handled in all phase
+  target->AddEventListener(NS_LITERAL_STRING("mousedown"), (nsIDOMMouseListener*)mMenuBarListener, PR_TRUE);
+  target->AddEventListener(NS_LITERAL_STRING("mousedown"), (nsIDOMMouseListener*)mMenuBarListener, PR_FALSE);
   target->AddEventListener(NS_LITERAL_STRING("blur"), (nsIDOMFocusListener*)mMenuBarListener, PR_TRUE);   
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsMenuBarFrame::SetActive(PRBool aActiveFlag)
 {
@@ -458,15 +460,16 @@ nsMenuBarFrame::DestroyFrom(nsIFrame* aD
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
   if (pm)
     pm->SetActiveMenuBar(this, PR_FALSE);
 
   mTarget->RemoveEventListener(NS_LITERAL_STRING("keypress"), (nsIDOMKeyListener*)mMenuBarListener, PR_FALSE); 
   mTarget->RemoveEventListener(NS_LITERAL_STRING("keydown"), (nsIDOMKeyListener*)mMenuBarListener, PR_FALSE);  
   mTarget->RemoveEventListener(NS_LITERAL_STRING("keyup"), (nsIDOMKeyListener*)mMenuBarListener, PR_FALSE);
 
+  mTarget->RemoveEventListener(NS_LITERAL_STRING("mousedown"), (nsIDOMMouseListener*)mMenuBarListener, PR_TRUE);
   mTarget->RemoveEventListener(NS_LITERAL_STRING("mousedown"), (nsIDOMMouseListener*)mMenuBarListener, PR_FALSE);
   mTarget->RemoveEventListener(NS_LITERAL_STRING("blur"), (nsIDOMFocusListener*)mMenuBarListener, PR_TRUE);
 
   NS_IF_RELEASE(mMenuBarListener);
 
   nsBoxFrame::DestroyFrom(aDestructRoot);
 }
--- a/layout/xul/base/src/nsMenuBarListener.cpp
+++ b/layout/xul/base/src/nsMenuBarListener.cpp
@@ -81,17 +81,17 @@ NS_INTERFACE_MAP_END
 
 ////////////////////////////////////////////////////////////////////////
 
 PRInt32 nsMenuBarListener::mAccessKey = -1;
 PRUint32 nsMenuBarListener::mAccessKeyMask = 0;
 PRBool nsMenuBarListener::mAccessKeyFocuses = PR_FALSE;
 
 nsMenuBarListener::nsMenuBarListener(nsMenuBarFrame* aMenuBar) 
-  :mAccessKeyDown(PR_FALSE)
+  :mAccessKeyDown(PR_FALSE), mAccessKeyDownCanceled(PR_FALSE)
 {
   mMenuBarFrame = aMenuBar;
 }
 
 ////////////////////////////////////////////////////////////////////////
 nsMenuBarListener::~nsMenuBarListener() 
 {
 }
@@ -169,23 +169,25 @@ nsMenuBarListener::KeyUp(nsIDOMEvent* aK
   {
     // On a press of the ALT key by itself, we toggle the menu's 
     // active/inactive state.
     // Get the ascii key code.
     nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aKeyEvent);
     PRUint32 theChar;
     keyEvent->GetKeyCode(&theChar);
 
-    if (mAccessKeyDown && (PRInt32)theChar == mAccessKey)
+    if (mAccessKeyDown && !mAccessKeyDownCanceled &&
+        (PRInt32)theChar == mAccessKey)
     {
       // The access key was down and is now up, and no other
       // keys were pressed in between.
       ToggleMenuActiveState();
     }
-    mAccessKeyDown = PR_FALSE; 
+    mAccessKeyDown = PR_FALSE;
+    mAccessKeyDownCanceled = PR_FALSE;
 
     PRBool active = mMenuBarFrame->IsActive();
     if (active) {
       aKeyEvent->StopPropagation();
       aKeyEvent->PreventDefault();
       return NS_OK; // I am consuming event
     }
   }
@@ -239,19 +241,20 @@ nsMenuBarListener::KeyPress(nsIDOMEvent*
         nsKeyEvent* nativeKeyEvent = static_cast<nsKeyEvent*>(nativeEvent);
         if (nativeKeyEvent) {
           nsAutoTArray<PRUint32, 10> keys;
           nsContentUtils::GetAccessKeyCandidates(nativeKeyEvent, keys);
           hasAccessKeyCandidates = !keys.IsEmpty();
         }
       }
 
-      // Clear the access key flag unless we are pressing the access key.
-      if (keyCode != (PRUint32)mAccessKey)
-        mAccessKeyDown = PR_FALSE;
+      // Cancel the access key flag unless we are pressing the access key.
+      if (keyCode != (PRUint32)mAccessKey) {
+        mAccessKeyDownCanceled = PR_TRUE;
+      }
 
       if (IsAccessKeyPressed(keyEvent) && hasAccessKeyCandidates) {
         // Do shortcut navigation.
         // A letter was pressed. We want to see if a shortcut gets matched. If
         // so, we'll know the menu got activated.
         nsMenuFrame* result = mMenuBarFrame->FindMenuWithShortcut(keyEvent);
         if (result) {
           mMenuBarFrame->SetActive(PR_TRUE);
@@ -336,29 +339,30 @@ nsMenuBarListener::KeyDown(nsIDOMEvent* 
     return NS_OK;
 
   if (mAccessKey && mAccessKeyFocuses)
   {
     nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aKeyEvent);
     PRUint32 theChar;
     keyEvent->GetKeyCode(&theChar);
 
-    if (theChar == (PRUint32)mAccessKey && (GetModifiers(keyEvent) & ~mAccessKeyMask) == 0) {
+    if (!mAccessKeyDownCanceled && theChar == (PRUint32)mAccessKey &&
+        (GetModifiers(keyEvent) & ~mAccessKeyMask) == 0) {
       // No other modifiers can be down.
       // Especially CTRL.  CTRL+ALT == AltGR, and
       // we'll fuck up on non-US enhanced 102-key
       // keyboards if we don't check this.
       mAccessKeyDown = PR_TRUE;
     }
     else {
       // Some key other than the access key just went down,
       // so we won't activate the menu bar when the access
       // key is released.
 
-      mAccessKeyDown = PR_FALSE;
+      mAccessKeyDownCanceled = PR_TRUE;
     }
   }
 
   return NS_OK; // means I am NOT consuming event
 }
 
 ////////////////////////////////////////////////////////////////////////
 
@@ -370,27 +374,44 @@ nsMenuBarListener::Focus(nsIDOMEvent* aE
 
 ////////////////////////////////////////////////////////////////////////
 nsresult
 nsMenuBarListener::Blur(nsIDOMEvent* aEvent)
 {
   if (!mMenuBarFrame->IsMenuOpen() && mMenuBarFrame->IsActive()) {
     ToggleMenuActiveState();
     mAccessKeyDown = PR_FALSE;
+    mAccessKeyDownCanceled = PR_FALSE;
   }
   return NS_OK; // means I am NOT consuming event
 }
   
 ////////////////////////////////////////////////////////////////////////
 nsresult 
 nsMenuBarListener::MouseDown(nsIDOMEvent* aMouseEvent)
 {
+  // NOTE: MouseDown method listens all phases
+
+  // Even if the mousedown event is canceled, it means the user don't want
+  // to activate the menu.  Therefore, we need to record it at capturing (or
+  // target) phase.
+  if (mAccessKeyDown) {
+    mAccessKeyDownCanceled = PR_TRUE;
+  }
+
+  PRUint16 phase = 0;
+  nsresult rv = aMouseEvent->GetEventPhase(&phase);
+  NS_ENSURE_SUCCESS(rv, rv);
+  // Don't do anything at capturing phase, any behavior should be cancelable.
+  if (phase == nsIDOMEvent::CAPTURING_PHASE) {
+    return NS_OK;
+  }
+
   if (!mMenuBarFrame->IsMenuOpen() && mMenuBarFrame->IsActive())
     ToggleMenuActiveState();
-  mAccessKeyDown = PR_FALSE;
 
   return NS_OK; // means I am NOT consuming event
 }
 
 ////////////////////////////////////////////////////////////////////////
 nsresult 
 nsMenuBarListener::MouseUp(nsIDOMEvent* aMouseEvent)
 {
--- a/layout/xul/base/src/nsMenuBarListener.h
+++ b/layout/xul/base/src/nsMenuBarListener.h
@@ -86,16 +86,19 @@ protected:
 
   static PRUint32 GetModifiers(nsIDOMKeyEvent* event);
 
   // This should only be called by the nsMenuBarListener during event dispatch,
   // thus ensuring that this doesn't get destroyed during the process.
   void ToggleMenuActiveState();
 
   nsMenuBarFrame* mMenuBarFrame; // The menu bar object.
-  PRBool mAccessKeyDown;         // Whether or not the ALT key is currently down.
+  // Whether or not the ALT key is currently down.
+  PRPackedBool mAccessKeyDown;
+  // Whether or not the ALT key down is canceled by other action.
+  PRPackedBool mAccessKeyDownCanceled;
   static PRBool mAccessKeyFocuses; // Does the access key by itself focus the menubar?
   static PRInt32 mAccessKey;     // See nsIDOMKeyEvent.h for sample values
   static PRUint32 mAccessKeyMask;// Modifier mask for the access key
 };
 
 
 #endif
--- a/toolkit/content/tests/widgets/window_menubar.xul
+++ b/toolkit/content/tests/widgets/window_menubar.xul
@@ -548,16 +548,30 @@ var popupTests = [
   events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
   test: function() { synthesizeKey("VK_F10", { }); },
 },
 {
   testname: "Deactivate menubar with alt key",
   condition: function() { return (navigator.platform.indexOf("Win") == 0) },
   events: [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu"  ],
   test: function() { synthesizeKey("VK_ALT", { }); },
+},
+{
+  testname: "Don't activate menubar with mousedown during alt key auto-repeat",
+  test: function() {
+    synthesizeKey("VK_ALT", { type: "keydown" });
+    synthesizeMouse(document.getElementById("menubar"), 8, -30, { type: "mousedown", altKey: true });
+    synthesizeKey("VK_ALT", { type: "keydown" });
+    synthesizeMouse(document.getElementById("menubar"), 8, -30, { type: "mouseup", altKey: true });
+    synthesizeKey("VK_ALT", { type: "keydown" });
+    synthesizeKey("VK_ALT", { type: "keyup" });
+  },
+  result: function (testname) {
+    checkActive(document.getElementById("menubar"), "", testname);
+  }
 }
 
 ];
 
 ]]>
 </script>