1115964 - Context menu items like "Remove from history" and "Open containing folder" fail to work, and the download loses its highlight

Status: RESOLVED WONTFIX

(Firefox :: Downloads Panel, defect)

Description

[Nicolas Barbulesco]

Firefox 34.0.5 Mac, on Mac OS X 10.6.8. 
I have a download that is finished. 
I click on the blue arrow. 
The Downloads bubble appears. It has one download. 
I place my mouse over my download. It becomes blue. 
I right-click my download. 
I choose "Remove from history". 

* Expected result : * 
The download was blue before the right click. It remains blue during the whole right click, wherever the mouse goes. When I release the right click, the download goes away.

* Actual result : * 
During the right click, when the mouse goes away from the download, the download becomes back white. When I release the right click, the download remains. I try again… Finally, at the 5th try, the removal works. It seems that, for the removal to work, the mouse must be over the download when I release the right click — which is difficult because of the height of the contextual menu.

Thank you for correcting this bug.

Nicolas B.

Comment 1

[Alice0775 White]

I can reproduce on Windows7

Comment 2

[Alice0775 White]

About #1 command "Remove from history" fails to work:
Pushlog
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3607139bd503&tochange=25c2aaee8acc

Regressed by:
63c003375416	Marco Bonardo — Bug 851519 - Keyboard commands may executed on the wrong download in the panel r=Mano

Comment 3

[Marco Bonardo [:mak]]

So the gesture you are trying is right-button down on a download and release on the Remove From History contextual menuitem?
Does it work if you just right click on the download and then left click on Remove From History?

Comment 4

[Alice0775 White]

STR
1. Download something
2. Left mouse click download toolbar button so that download panel pops up
3. Right mouse click a entry of download (you should click the right-bottom part of the highlighted area so that context menu does not overlap with highlighted area as much as possible) 
4. Left click "Remove From History" by mouse (mouse pointer should be positioned outside of the arrow panel)

Actual Results:
"Remove From History" does nothing.
"Open containing folder", "Go To Download Page" and "Copt Download Link" also fail.

Comment 6

[Gingerbread Man]

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150419030206

Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150419030206

On Windows 7, the downloaded item remained highlighted. It only lost its highlight in Linux Mint KDE 17.1.

Comment 7

[George Pîrlea]

Attachment: 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.

Review commit: https://reviewboard.mozilla.org/r/101010/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/101010/

Comment 8

[George Pîrlea]

Comment on attachment 8821934 [details]
Bug 1115964 - Inhibit download item selection if a context menu is open

https://reviewboard.mozilla.org/r/101008/#review101552

::: browser/components/downloads/content/downloads.js:1032
(Diff revision 1)
>    /**
>     * Mouse listeners to handle selection on hover.
>     */
>    onDownloadMouseOver(aEvent) {
> -    if (aEvent.target.parentNode == this.richListBox) {
> +    if (!this.contextMenuOpen &&
> +        aEvent.target.parentNode == this.richListBox) {

I might have slightly messed up the submission here. Lines 1032 and 1033 here are relative to https://reviewboard.mozilla.org/r/101004, rather than the original repo.

If you were to apply this changeset, you'd have to apply that one first.

Comment 9

[George Pîrlea]

This is related to bug 991965.

Comment 10

[George Pîrlea]

Attachment: context-menu-before-after-inhibit.ogv

Paolo asked in bug 991965 (comment 17 there) to better explain what problem my patch for this bug solves:

In the Downloads Panel, if you open a context menu for an item (by right-clicking it), you can still hover over and select other items. In this case, when you click 'Remove From History', you may (unintuitively) remove an item different than the one you opened the context menu on.

The attached screencast shows selection behaviour with an open context menu before and after the patch.

Comment 11

[:Paolo Amadini]

Comment on attachment 8821934 [details]
Bug 1115964 - Inhibit download item selection if a context menu is open

https://reviewboard.mozilla.org/r/101010/#review106700

Reviewed this in bug 991965 comment 19.

::: browser/components/downloads/content/downloads.js:1030
(Diff revision 1)
>    onDownloadMouseOver(aEvent) {
> -    if (aEvent.target.parentNode == this.richListBox) {
> +    if (!this.contextMenuOpen &&
> +        aEvent.target.parentNode == this.richListBox) {

As noted in the other bug, we should fix the incorrect use of originalTarget in this function too.

::: browser/components/downloads/content/downloadsOverlay.xul:63
(Diff revision 1)
> +                  onpopupshown="DownloadsView.onContextPopupShown(event);"
> +                  onpopuphidden="DownloadsView.onContextPopupHidden(event);"

nit: there's an extra space, ensure these are aligned correctly.

Comment 12

[:Paolo Amadini]

Over in bug 991965, we noted that when the context menu is open, clicking anywhere else just closes the menu. Thus, we might suppress the normal mouse hover highlight in the Downloads Panel while the context menu is open. Instead, we could replace it with an item-wide highlight, which will be "locked" to the item that was right-clicked originally.

Carol, any preference for how this highlight could look like, or any recommendation against implementing it? I'm thinking of using the current dimmed style, but across the entire download item.

This is conceptually identical to what the system file managers do, although on Windows 7 the highlight for the clicked item looks more like a mouse hover highlight, while on Mac OS X 10.9 it looks more like a focus ring.

Let me know if anything is unclear!

Comment 13

[Carol Huang [:Carol]]

Hi Paolo,
I checked other items on the menu panel, once the context menu is open, the hover highlight will only on the mouse-over item. Therefore, in my opinion, to be consistent, we could keep the current behavior. We don't need item-wide highlight or other highlight for the item was right-clicked originally.
I hope it makes sense.
Let me know if you have any question or something unclear! thank you :)

Comment 14

[:Paolo Amadini]

I see what you mean, for example the tab strip has the same issue. If I invoke the context menu on any tab that is not selected, then there is no persistent indication of which tab I invoked the menu on.

This is possibly something to improve in general in the user interface of the browser, but I'll close this bug that is specific to the Downloads Panel for now. If we fix the issue with the more important user interface elements like the tab strip, then we can revisit this for the Downloads Panel as well.