Bug 1318790 - Don't use mouseenter and mouseleave in searchbar binding. r=florian
authorDrew Willcoxon <adw@mozilla.com>
Mon, 21 Nov 2016 09:50:37 -0800
changeset 323606 015513b6fd0f87ef8c2ccedfcf727f47f67ac6ee
parent 323605 4df1bb338f6d581509ac3917ee79dcc3997bdfa6
child 323607 706a36d583ce641dd0f5bd7c3d89cd4adf19d779
push id30980
push usercbook@mozilla.com
push dateTue, 22 Nov 2016 14:47:06 +0000
treeherdermozilla-central@d5d357b7ca30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1318790
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 1318790 - Don't use mouseenter and mouseleave in searchbar binding. r=florian MozReview-Commit-ID: 5SKzyEC0fVi
browser/components/search/content/search.xml
browser/components/search/test/browser_tooManyEnginesOffered.js
browser/themes/osx/searchbar.css
--- a/browser/components/search/content/search.xml
+++ b/browser/components/search/content/search.xml
@@ -2070,16 +2070,34 @@
           return;
         }
         // Required to receive click events from the buttons on Linux.
         event.preventDefault();
       ]]></handler>
 
       <handler event="mousemove"><![CDATA[
         let target = event.originalTarget;
+
+        // Handle mouseover on the add-engine menu button and its popup items.
+        if (target.getAttribute("anonid") == "addengine-menu-button" ||
+            (target.localName == "menuitem" &&
+             target.classList.contains("addengine-item"))) {
+          // Make the menu button visually selected.  It's highlighted in the
+          // CSS when the popup is open, but the popup doesn't open until a
+          // short timeout has elapsed.  Making the button visually selected now
+          // provides better feedback to the user.
+          let menuButton = document.getAnonymousElementByAttribute(
+            this, "anonid", "addengine-menu-button"
+          );
+          this._changeVisuallySelectedButton(menuButton);
+          this._addEngineMenuShouldBeOpen = true;
+          this._resetAddEngineMenuTimeout();
+          return;
+        }
+
         if (target.localName != "button")
           return;
 
         // Ignore mouse events when the context menu is open.
          if (this._ignoreMouseEvents)
            return;
 
         let isOneOff =
@@ -2087,36 +2105,32 @@
           !target.classList.contains("dummy");
         if (isOneOff ||
             target.classList.contains("addengine-item") ||
             target.classList.contains("search-setting-button")) {
           this._changeVisuallySelectedButton(target);
         }
       ]]></handler>
 
-      <handler event="mouseenter"><![CDATA[
+      <handler event="mouseout"><![CDATA[
+
         let target = event.originalTarget;
-        if (target.getAttribute("anonid") == "addengine-menu-button") {
-          this._addEngineMenuShouldBeOpen = true;
-          this._resetAddEngineMenuTimeout();
-          return;
-        }
-      ]]></handler>
 
-      <handler event="mouseleave"><![CDATA[
-        let target = event.originalTarget;
-        if (target.getAttribute("anonid") == "addengine-menu-button") {
+        // Handle mouseout on the add-engine menu button and its popup items.
+        if (target.getAttribute("anonid") == "addengine-menu-button" ||
+            (target.localName == "menuitem" &&
+             target.classList.contains("addengine-item"))) {
+          // The menu button will appear selected since the mouse is either over
+          // it or over one of the menu items in the popup.  Make it unselected.
+          this._changeVisuallySelectedButton(null);
           this._addEngineMenuShouldBeOpen = false;
           this._resetAddEngineMenuTimeout();
           return;
         }
-      ]]></handler>
 
-      <handler event="mouseout"><![CDATA[
-        let target = event.originalTarget;
         if (target.localName != "button") {
           return;
         }
 
         // Don't deselect the current button if the context menu is open.
         if (this._ignoreMouseEvents)
           return;
 
--- a/browser/components/search/test/browser_tooManyEnginesOffered.js
+++ b/browser/components/search/test/browser_tooManyEnginesOffered.js
@@ -26,17 +26,17 @@ add_task(function* test() {
   let items = getOpenSearchItems();
   Assert.equal(items.length, 1, "A single button")
   let menuButton = items[0];
   Assert.equal(menuButton.type, "menu", "A menu button");
 
   // Mouse over the menu button to open it.
   let buttonPopup = menuButton.firstChild;
   promise = promiseEvent(buttonPopup, "popupshown");
-  EventUtils.synthesizeMouse(menuButton, 5, 5, { type: "mouseover" });
+  EventUtils.synthesizeMouse(menuButton, 5, 5, { type: "mousemove" });
   yield promise;
 
   Assert.ok(menuButton.open, "Submenu should be open");
 
   // Check the engines inside the submenu.
   Assert.equal(buttonPopup.childNodes.length, 6, "Expected number of engines");
   for (let i = 0; i < buttonPopup.childNodes.length; i++) {
     let item = buttonPopup.childNodes[i];
--- a/browser/themes/osx/searchbar.css
+++ b/browser/themes/osx/searchbar.css
@@ -242,18 +242,17 @@
   border-top: 1px solid var(--panel-separator-color);
 }
 
 .addengine-item[selected] {
   background-color: Highlight;
   color: HighlightText;
 }
 
-.addengine-item[type=menu][selected],
-.addengine-item[type=menu][open] {
+.addengine-item[type=menu][selected] {
   color: inherit;
   background-color: var(--arrowpanel-dimmed-further);
 }
 
 .addengine-icon {
   width: 16px;
 }