Bug 991965 - Use event.target, inhibit item selection if context menu is open
1) In the Downloads Panel, the item with the hover effect applied to it was not
always the actual 'selectedItem'. This was due to an error in how
'onDownloadMouseOver()' events were processed – 'aEvent.orginalTarget' doesn't
take retargets into account, so hovering over an item would sometimes not
trigger an actual selection.
This is what the reporter of
bug 991965 describes: removing an item from the
download history fails to work "just because you didn't click in some magical
place". ('selectedItem' was -1)
Fixed by using 'aEvent.target' instead of 'aEvent.originalTarget'.
2) 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.
This commit DOES NOT include the necessary style changes. At this point, the
selection UI is completely broken!
MozReview-Commit-ID: DLIAFNcs33N
--- 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: 5,
/**
+ * Indicates whether there is an open contextMenu for a download item.
+ */
+ 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
@@ -995,30 +1000,56 @@ const DownloadsView = {
}
if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN) {
goDoCommand("downloadsCmd_doDefault");
}
},
/**
+ * Event handlers to keep track of context menu state (open/closed) for
+ * download items.
+ */
+ 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;
+ },
+
+ /**
* Mouse listeners to handle selection on hover.
*/
onDownloadMouseOver(aEvent) {
- if (aEvent.originalTarget.parentNode == this.richListBox) {
- this.richListBox.selectedItem = aEvent.originalTarget;
+ 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.target.parentNode == this.richListBox) {
// If the destination element is outside of the richlistitem, clear the
// selection.
let element = aEvent.relatedTarget;
- while (element && element != aEvent.originalTarget) {
+ while (element && element != aEvent.target) {
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;"