815691 - The panel's context menu doesn't change after clicking on a different item

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect)

Description

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20.0 Firefox/20.0
Build ID: 20121127030907

Prerequisites:
A failed download and a complete download are needed in the downloads panel.

Steps to reproduce:
1. Open the Downloads panel
2. Right click on the failed download.
3. Right click on the completed download.

Expected results:
The context menu for the 2 downloads item are different (one is for a failed download and the other for a completed one)

Actual results:
The context menu is the same for both downloads.
Please see the screencast: http://screencast.com/t/3b45svbvwDU

Note: This is reproducible only on Windows (not on Ubuntu or Mac OS X).

Comment 1

[Simona Badau, Desktop QA]

Last good nightly: 2012-10-30
First bad nightly: 2012-10-31

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e19e170d2f6d&tochange=bed18790882f

Comment 2

[Marco Bonardo [:mak]]

Thanks Simona, looks related to bug 746775

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Ew - this has a nasty side-effect:

1) Right click on the failed download
2) Right click on the complete download, and choose to remove it from the list

What happens?

The failed download is removed instead of the complete download.

What's expected?

The completed download should be removed.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Hey Neil - do you know why the context menus might not be opening for the right items here? (See the screencast from comment 0 for an example).

Comment 5

[Neil Deakin]

Is the current item being set properly? What are downloadsListBox.currentIndex and downloadsListBox.selectedIndex when the context menu is opened?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

(In reply to Neil Deakin from comment #5)
> Is the current item being set properly? What are
> downloadsListBox.currentIndex and downloadsListBox.selectedIndex when the
> context menu is opened?

Hm - no, when a context menu is open, and I right-click on another item, the currentIndex and selectedIndex are *not* updated.

Comment 7

[Neil Deakin]

OK, so the selection should happen at http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/listbox.xml#983

On Windows, perhaps the mousedown event isn't happening before the contextmenu event?

Comment 8

[Mike Conley (:mconley) (:⚙️)]

(In reply to Neil Deakin from comment #7)
> OK, so the selection should happen at
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/
> listbox.xml#983
> 
> On Windows, perhaps the mousedown event isn't happening before the
> contextmenu event?

If I right-click on a list item when there is no context menu already open, I get the mousedown event, and then the contextmenu event.

With that context menu still open, if I right click on another item, I just get the contextmenu event. No mousedown event is fired.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

So, is the correct course of action to relax our rule somewhat so that if a context window is open, we don't allow left-clicks through, but we *do* allow right-clicks through to the underlying panel?

Comment 10

[Neil Deakin]

Attachment: Possible patch

This might fix the issue. I'm sure it will cause some problem somewhere, but I can't see anything wrong offhand.

I tested bug 404766 which added this change originally and don't see a problem, but it would be good for someone else to confirm.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 690538 [details] [diff] [review]
Possible patch

I can confirm that this doesn't re-open bug 404766, or bug 746775, and fixes this case.

Comment 12

[Marco Bonardo [:mak]]

Neil, may we try to take the above patch and see how it behaves in real life? In case of problems it's a trivial backout to do and since the kind of issues it may cause are in the UI, they should be easily repeatable to find a reg-range to this and may tell us more.

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b807fead05b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8b807fead05b

Comment 15

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20121218 Firefox/20.0
Build ID: 20121218030803

Verified as fixed on the latest Nightly - verified that the context menu for the 2 downloads items (the one that is completed and the one that is failed) are different and that the scenario from step 3 is no longer reproducible.