Bug 1318790 - Don't use mouseenter and mouseleave in searchbar binding. r?florian draft
authorDrew Willcoxon <adw@mozilla.com>
Mon, 21 Nov 2016 09:50:37 -0800
changeset 442086 f97d3d3f409f965f328664312648eef91d173573
parent 441564 954062f90f04afc216a7d00355375b02c78a77e3
child 537693 df65a30691d1fd4c2bd802bb410f1658d620ab45
push id36579
push userdwillcoxon@mozilla.com
push dateMon, 21 Nov 2016 17:51:42 +0000
reviewersflorian
bugs1318790
milestone52.0a1
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
@@ -2065,16 +2065,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 =
@@ -2082,36 +2100,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
@@ -219,18 +219,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;
 }