Bug 1115964 - Inhibit download item selection if a context menu is open draft
authorGeorge Pîrlea <george@dranov.ro>
Mon, 26 Dec 2016 23:04:42 +0000
changeset 453964 6b211995a4c2849f9d53aefcfa5eddce5c1e6a6b
parent 453948 ae1266af1faa3f7a574717913baf6e99964bc103
child 540568 7743fe20336b92258f6225fcd405371f7848cb48
push id39779
push userbmo:george@dranov.ro
push dateMon, 26 Dec 2016 23:10:14 +0000
bugs1115964
milestone53.0a1
Bug 1115964 - Inhibit download item selection if a context menu is open In the Downloads Panel, right-clicking an item opens a context menu that lets you act on the selected item. However, if you unintentionally hover over a different item when the context menu is open, the selection changes - you may, for example, 'Remove From History' a different, unintended item. To fix this, we inhibit item selection when a context menu is open for a download item. We also make sure that the 'hover' effect is always applied on the select item, even if the mouse isn't hovering over it. MozReview-Commit-ID: 1OR8KilNi5Z
browser/components/downloads/content/downloads.js
browser/components/downloads/content/downloadsOverlay.xul
browser/themes/shared/downloads/downloads.inc.css
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -718,16 +718,21 @@ const DownloadsView = {
   //// Functions handling download items in the list
 
   /**
    * Maximum number of items shown by the list at any given time.
    */
   kItemCountLimit: 3,
 
   /**
+   * Indicates whether there is an open contextMenu for a downloadItem
+   */
+  contextMenuOpen: false,
+
+  /**
    * Indicates whether we are still loading downloads data asynchronously.
    */
   loading: false,
 
   /**
    * Ordered array of all Download objects.  We need to keep this array because
    * only a limited number of items are shown at once, and if an item that is
    * currently visible is removed from the list, we might need to take another
@@ -938,16 +943,36 @@ const DownloadsView = {
     }
     this._visibleViewItems.delete(download);
     this._itemsForElements.delete(element);
   },
 
   //////////////////////////////////////////////////////////////////////////////
   //// User interface event functions
 
+  onContextPopupShown(aEvent) {
+    // Ignore events raised by nested popups.
+    if (aEvent.target != aEvent.currentTarget) {
+      return;
+    }
+
+    DownloadsCommon.log("Context menu has shown.");
+    this.contextMenuOpen = true;
+  },
+
+  onContextPopupHidden(aEvent) {
+    // Ignore events raised by nested popups.
+    if (aEvent.target != aEvent.currentTarget) {
+      return;
+    }
+
+    DownloadsCommon.log("Context menu has hidden.");
+    this.contextMenuOpen = false;
+  },
+
   /**
    * Helper function to do commands on a specific download item.
    *
    * @param aEvent
    *        Event object for the event being handled.  If the event target is
    *        not a richlistitem that represents a download, this function will
    *        walk up the parent nodes until it finds a DOM node that is.
    * @param aCommand
@@ -998,23 +1023,25 @@ const DownloadsView = {
       goDoCommand("downloadsCmd_doDefault");
     }
   },
 
   /**
    * Mouse listeners to handle selection on hover.
    */
   onDownloadMouseOver(aEvent) {
-    if (aEvent.target.parentNode == this.richListBox) {
+    if (!this.contextMenuOpen &&
+        aEvent.target.parentNode == this.richListBox) {
       this.richListBox.selectedItem = aEvent.target;
     }
   },
 
   onDownloadMouseOut(aEvent) {
-    if (aEvent.originalTarget.parentNode == this.richListBox) {
+    if (!this.contextMenuOpen &&
+        aEvent.originalTarget.parentNode == this.richListBox) {
       // If the destination element is outside of the richlistitem, clear the
       // selection.
       let element = aEvent.relatedTarget;
       while (element && element != aEvent.originalTarget) {
         element = element.parentNode;
       }
       if (!element) {
         this.richListBox.selectedIndex = -1;
--- a/browser/components/downloads/content/downloadsOverlay.xul
+++ b/browser/components/downloads/content/downloadsOverlay.xul
@@ -55,16 +55,18 @@
            orient="vertical"
            level="top"
            onpopupshown="DownloadsPanel.onPopupShown(event);"
            onpopuphidden="DownloadsPanel.onPopupHidden(event);">
       <!-- The following popup menu should be a child of the panel element,
            otherwise flickering may occur when the cursor is moved over the area
            of a disabled menu item that overlaps the panel.  See bug 492960. -->
       <menupopup id="downloadsContextMenu"
+                  onpopupshown="DownloadsView.onContextPopupShown(event);"
+                  onpopuphidden="DownloadsView.onContextPopupHidden(event);"
                  class="download-state">
         <menuitem command="downloadsCmd_pauseResume"
                   class="downloadPauseMenuItem"
                   label="&cmd.pause.label;"
                   accesskey="&cmd.pause.accesskey;"/>
         <menuitem command="downloadsCmd_pauseResume"
                   class="downloadResumeMenuItem"
                   label="&cmd.resume.label;"
--- a/browser/themes/shared/downloads/downloads.inc.css
+++ b/browser/themes/shared/downloads/downloads.inc.css
@@ -108,17 +108,17 @@
 toolbarseparator.downloadsDropmarkerSplitter {
   margin: 7px 0;
 }
 
 @item@ > toolbarseparator {
   margin: 10px 0;
 }
 
-@item@:hover > toolbarseparator,
+@item@[selected] > toolbarseparator,
 #downloadsFooter:hover toolbarseparator.downloadsDropmarkerSplitter,
 #downloadsFooter[showingdropdown] toolbarseparator {
   margin: 0;
 }
 
 .downloadsDropmarker {
   padding: 0 21px;
 }
@@ -268,42 +268,42 @@ richlistitem[type="download"] > .downloa
   fill: currentColor;
 }
 
 .downloadButton > .button-box > .button-text {
   margin: 0 !important;
   padding: 0;
 }
 
-@itemFinished@[exists]:hover > .downloadMainArea,
-@item@:not([verdict]):hover > .downloadButtonArea {
+@itemFinished@[exists][selected] > .downloadMainArea,
+@item@:not([verdict])[selected] > .downloadButtonArea {
   background-color: var(--arrowpanel-dimmed);
 }
 
-@itemFinished@[exists] .downloadMainArea:hover,
-@item@:not([verdict]) > .downloadButtonArea:hover,
-@item@[verdict]:hover {
+@itemFinished@[exists][selected] .downloadMainArea:hover,
+@item@:not([verdict])[selected] > .downloadButtonArea:hover,
+@item@[verdict][selected] {
   background-color: var(--arrowpanel-dimmed-further);
 }
 
-@itemFinished@[exists] > .downloadMainArea:hover:active,
-@item@:not([verdict]) > .downloadButtonArea:hover:active,
-@item@[verdict]:hover:active {
+@itemFinished@[exists][selected] > .downloadMainArea:hover:active,
+@item@:not([verdict])[selected] > .downloadButtonArea:hover:active,
+@item@[verdict][selected]:active {
   background-color: var(--arrowpanel-dimmed-even-further);
 }
 
 @item@[showingsubview] {
   background-color: Highlight;
   color: HighlightText;
   transition: background-color var(--panelui-subview-transition-duration),
               color var(--panelui-subview-transition-duration);
 }
 
-@item@[verdict="Malware"]:hover,
-@item@[verdict="Malware"]:hover:active,
+@item@[verdict="Malware"][selected],
+@item@[verdict="Malware"][selected]:active,
 @item@[verdict="Malware"][showingsubview] {
   background-color: #aa1b08;
   color: white;
 }
 
 /*** Button icons ***/
 
 .downloadIconCancel > .button-box > .button-icon {