Bug 1327908, on Windows, don't hide tooltips when a key is pressed. On other platforms, allow hiding tooltips when a key is pressed except when modifier keys are pressed alone, r=felipe
authorNeil Deakin <neil@mozilla.com>
Thu, 19 Jan 2017 13:57:11 -0500
changeset 375158 a12e66cb7bc27c0b8e1822a86b3e91763011e211
parent 375157 d6f80f3a2400de5b75083485732e8f740cc006f3
child 375159 c8db5ac137c544ab0d018566e3fe137f7601c523
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfelipe
bugs1327908
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1327908, on Windows, don't hide tooltips when a key is pressed. On other platforms, allow hiding tooltips when a key is pressed except when modifier keys are pressed alone, r=felipe
docshell/base/nsDocShellTreeOwner.cpp
layout/xul/nsXULTooltipListener.cpp
toolkit/content/tests/chrome/window_tooltip.xul
--- a/docshell/base/nsDocShellTreeOwner.cpp
+++ b/docshell/base/nsDocShellTreeOwner.cpp
@@ -68,16 +68,17 @@
 #include "nsView.h"
 #include "nsIDOMDragEvent.h"
 #include "nsIConstraintValidation.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/EventListenerManager.h"
 #include "mozilla/dom/Event.h" // for nsIDOMEvent::InternalDOMEvent()
 #include "mozilla/dom/File.h" // for input type=file
 #include "mozilla/dom/FileList.h" // for input type=file
+#include "mozilla/TextEvents.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 // A helper routine that navigates the tricky path from a |nsWebBrowser| to
 // a |EventTarget| via the window root and chrome event handler.
 static nsresult
 GetDOMEventTarget(nsWebBrowser* aInBrowser, EventTarget** aTarget)
@@ -1110,19 +1111,21 @@ ChromeTooltipListener::AddChromeListener
 // for mouseExit, "mouse motion" for mouseMove, and "key" for keyDown. As we
 // add the listeners, keep track of how many succeed so we can clean up
 // correctly in Release().
 NS_IMETHODIMP
 ChromeTooltipListener::AddTooltipListener()
 {
   if (mEventTarget) {
     nsresult rv = NS_OK;
+#ifndef XP_WIN
     rv = mEventTarget->AddSystemEventListener(NS_LITERAL_STRING("keydown"),
                                               this, false, false);
     NS_ENSURE_SUCCESS(rv, rv);
+#endif
     rv = mEventTarget->AddSystemEventListener(NS_LITERAL_STRING("mousedown"),
                                               this, false, false);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = mEventTarget->AddSystemEventListener(NS_LITERAL_STRING("mouseout"),
                                               this, false, false);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = mEventTarget->AddSystemEventListener(NS_LITERAL_STRING("mousemove"),
                                               this, false, false);
@@ -1151,19 +1154,21 @@ ChromeTooltipListener::RemoveChromeListe
 }
 
 // Unsubscribe from all the various tooltip events that we were listening to.
 NS_IMETHODIMP
 ChromeTooltipListener::RemoveTooltipListener()
 {
   if (mEventTarget) {
     nsresult rv = NS_OK;
+#ifndef XP_WIN
     rv = mEventTarget->RemoveSystemEventListener(NS_LITERAL_STRING("keydown"),
                                                  this, false);
     NS_ENSURE_SUCCESS(rv, rv);
+#endif
     rv = mEventTarget->RemoveSystemEventListener(NS_LITERAL_STRING("mousedown"),
                                                  this, false);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = mEventTarget->RemoveSystemEventListener(NS_LITERAL_STRING("mouseout"),
                                                  this, false);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = mEventTarget->RemoveSystemEventListener(NS_LITERAL_STRING("mousemove"),
                                                  this, false);
@@ -1176,19 +1181,25 @@ ChromeTooltipListener::RemoveTooltipList
 }
 
 NS_IMETHODIMP
 ChromeTooltipListener::HandleEvent(nsIDOMEvent* aEvent)
 {
   nsAutoString eventType;
   aEvent->GetType(eventType);
 
-  if (eventType.EqualsLiteral("keydown") ||
-      eventType.EqualsLiteral("mousedown")) {
+  if (eventType.EqualsLiteral("mousedown")) {
     return HideTooltip();
+  } else if (eventType.EqualsLiteral("keydown")) {
+    WidgetKeyboardEvent* keyEvent = aEvent->WidgetEventPtr()->AsKeyboardEvent();
+    if (!keyEvent->IsModifierKeyEvent()) {
+      return HideTooltip();
+    }
+
+    return NS_OK;
   } else if (eventType.EqualsLiteral("mouseout")) {
     // Reset flag so that tooltip will display on the next MouseMove
     mTooltipShownOnce = false;
     return HideTooltip();
   } else if (eventType.EqualsLiteral("mousemove")) {
     return MouseMove(aEvent);
   }
 
--- a/layout/xul/nsXULTooltipListener.cpp
+++ b/layout/xul/nsXULTooltipListener.cpp
@@ -24,16 +24,17 @@
 #endif
 #include "nsIRootBox.h"
 #include "nsIBoxObject.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Event.h" // for nsIDOMEvent::InternalDOMEvent()
 #include "mozilla/dom/BoxObject.h"
+#include "mozilla/TextEvents.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 nsXULTooltipListener* nsXULTooltipListener::mInstance = nullptr;
 
 //////////////////////////////////////////////////////////////////////////
 //// nsISupports
@@ -222,24 +223,33 @@ nsXULTooltipListener::MouseMove(nsIDOMEv
 }
 
 NS_IMETHODIMP
 nsXULTooltipListener::HandleEvent(nsIDOMEvent* aEvent)
 {
   nsAutoString type;
   aEvent->GetType(type);
   if (type.EqualsLiteral("DOMMouseScroll") ||
-      type.EqualsLiteral("keydown") ||
       type.EqualsLiteral("mousedown") ||
       type.EqualsLiteral("mouseup") ||
       type.EqualsLiteral("dragstart")) {
     HideTooltip();
     return NS_OK;
   }
 
+  if (type.EqualsLiteral("keydown")) {
+    // Hide the tooltip if a non-modifier key is pressed.
+    WidgetKeyboardEvent* keyEvent = aEvent->WidgetEventPtr()->AsKeyboardEvent();
+    if (!keyEvent->IsModifierKeyEvent()) {
+      HideTooltip();
+    }
+
+    return NS_OK;
+  }
+
   if (type.EqualsLiteral("popuphiding")) {
     DestroyTooltip();
     return NS_OK;
   }
 
   // Note that mousemove, mouseover and mouseout might be
   // fired even during dragging due to widget's bug.
   nsCOMPtr<nsIDragService> dragService =
@@ -425,18 +435,21 @@ nsXULTooltipListener::ShowTooltip()
         // AddSystemEventListener(), the event target sets it to TRUE if the
         // target is in content.
         doc->AddSystemEventListener(NS_LITERAL_STRING("DOMMouseScroll"),
                                     this, true);
         doc->AddSystemEventListener(NS_LITERAL_STRING("mousedown"),
                                     this, true);
         doc->AddSystemEventListener(NS_LITERAL_STRING("mouseup"),
                                     this, true);
+#ifndef XP_WIN
+        // On Windows, key events don't close tooltips.
         doc->AddSystemEventListener(NS_LITERAL_STRING("keydown"),
                                     this, true);
+#endif
       }
       mSourceNode = nullptr;
     }
   }
 
   return NS_OK;
 }
 
@@ -664,17 +677,19 @@ nsXULTooltipListener::DestroyTooltip()
     nsCOMPtr<nsIDocument> doc = currentTooltip->GetComposedDoc();
     if (doc) {
       // remove the mousedown and keydown listener from document
       doc->RemoveSystemEventListener(NS_LITERAL_STRING("DOMMouseScroll"), this,
                                      true);
       doc->RemoveSystemEventListener(NS_LITERAL_STRING("mousedown"), this,
                                      true);
       doc->RemoveSystemEventListener(NS_LITERAL_STRING("mouseup"), this, true);
+#ifndef XP_WIN
       doc->RemoveSystemEventListener(NS_LITERAL_STRING("keydown"), this, true);
+#endif
     }
 
     // remove the popuphidden listener from tooltip
     currentTooltip->RemoveSystemEventListener(NS_LITERAL_STRING("popuphiding"), this, false);
   }
 
   // kill any ongoing timers
   KillTooltipTimer();
--- a/toolkit/content/tests/chrome/window_tooltip.xul
+++ b/toolkit/content/tests/chrome/window_tooltip.xul
@@ -258,18 +258,71 @@ var popupTests = [
     var buttonrect = document.getElementById("withtooltip").getBoundingClientRect();
     var rect = document.getElementById("thetooltip").getBoundingClientRect();
     var popupstyle = window.getComputedStyle(document.getElementById("thetooltip"), "");
 
     is(Math.round(rect.y + rect.height),
        Math.round(buttonrect.top + 4 - parseFloat(popupstyle.marginTop)),
        testname + " position of tooltip above button");
   }
+},
+{
+  testname: "open tooltip for keyclose",
+  events: [ "popupshowing thetooltip", "popupshown thetooltip" ],
+  test: function() {
+    gButton = document.getElementById("withtooltip");
+    gExpectedTriggerNode = gButton;
+    disableNonTestMouse(true);
+    synthesizeMouse(gButton, 2, 2, { type: "mouseover" });
+    synthesizeMouse(gButton, 4, 4, { type: "mousemove" });
+    synthesizeMouse(gButton, 6, 6, { type: "mousemove" });
+    disableNonTestMouse(false);
+  },
+},
+{
+  testname: "close tooltip with modifiers",
+  test: function() {
+    // Press all of the modifiers; the tooltip should remain open on all platforms.
+    synthesizeKey("VK_SHIFT", {});
+    synthesizeKey("VK_CONTROL", {});
+    synthesizeKey("VK_ALT", {});
+    synthesizeKey("VK_META", {});
+  },
+  result: function() {
+    is(document.getElementById("thetooltip").state, "open", "tooltip still open after modifiers pressed")
+  }
+},
+{
+  testname: "close tooltip with key",
+  events: function() {
+    if (navigator.platform.indexOf("Win") > -1) {
+      return [];
+    }
+    else {
+      return [ "popuphiding thetooltip", "popuphidden thetooltip",
+               "DOMMenuInactive thetooltip" ];
+    }
+  },
+  test: function() {
+    synthesizeKey("a", {});
+  },
+  result: function() {
+    let expectedState = (navigator.platform.indexOf("Win") > -1) ? "open" : "closed";
+    is(document.getElementById("thetooltip").state, expectedState, "tooltip closed after key pressed")
+  }
+},
+{
+  testname: "close tooltip with hidePopup again",
+  condition: function() { return navigator.platform.indexOf("Win") > -1; },
+  events: [ "popuphiding thetooltip", "popuphidden thetooltip",
+            "DOMMenuInactive thetooltip" ],
+  test: function() {
+    document.getElementById("thetooltip").hidePopup();
+  },
 }
-
 ];
 
 var waitSteps = 0;
 function moveWindowTo(x, y, callback, arg)
 {
   if (!waitSteps) {
     oldx = window.screenX;
     oldy = window.screenY;