246279 - Double click should be mouse button specific

Status: VERIFIED DUPLICATE of bug 244692

(Toolkit :: Add-ons Manager, defect)

Description

[Ben Basson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8

Clicks on extensions should probably be mouse button specific

Reproducible: Always
Steps to Reproduce:
1. Left click then quickly right click on any extension in your list.
Actual Results:  
Options dialog is opened.

Expected Results:  
Extension should have been selected and the context menu shown.

Right clicking once on any extension will highlight it, as well as showing the
context menu. While this is nice, left clicking first should probably not cause
a double click action.

Comment 1

[Ben Basson]

Attachment: Check event button

Should solve this minor annoyance

Comment 2

[Michael Lefevre]

I think "trivial" may be overstating the important of this bug, but it does
indeed seem to happen :)  Confirming.

Requesting review from nobody in particular isn't going to get you anywhere
(unless a reviewer happens to read this bug, in which case they could do it
anyway). You should pick a person that you think might do the review and request
it from them.

Comment 3

[Ben Basson]

Ooops, my bad. This is my first patch, so I was a little unsure of the
procedure. Requested that Ben review this, since the EM is his work. I think you
could be on to something - maybe we need something below trivial ;)

Comment 4

[Mike Connor [:mconnor]]

Comment on attachment 150493 [details] [diff] [review]
Check event button

yeah, we need to do this is multiple places

r=[e-mail]

Comment 5

[:Gavin Sharp [email: [e-mail]]]

This is awfully similar to bug 244692, which covers not only the extension
manager but also other GUI internals.

Comment 6

[Ben Basson]

mconner pointed that bug out to me, I'm going to patch the remaining ondblclick
events shortly. Regarding the patch for this bug, should it be != 0, rather than
== 2?

Comment 7

[Unknown W. Brackets]

(In reply to comment #6)
> mconner pointed that bug out to me, I'm going to patch the remaining ondblclick
> events shortly. Regarding the patch for this bug, should it be != 0, rather than
> == 2?

I like the solution I gave in my patch. (although it was for a different area
altogether...)  Why?

1. It doesn't add arguments to any functions.  This makes it less likely to
cause problems, as adding arguments could cause weird conflicts, in cases.

2. It does it in the event itself, where it is most logical, and doesn't affect
code that might not need it. (eg. calling the same function, but without an
event at all!)

3. It checks that the button is 0. (meaning left, not right or *middle* which
also does it.)

But, then, I wrote it that way and so of course I will think it's better.

As far as the correct behavior on this... looks none to certain to me.

"dblclick
     The dblclick event occurs when the pointing device button is double
     clicked over an element. This attribute may be used with most elements."

It doesn't actually say you can double click anything but "the pointing device
button", which would lead me to believe it should only work for left clicks. 
But it looks meant to be rather user agent specific.

-[Unknown]

Comment 8

[Ben Basson]

The only reason I did this in the scripting was because in some cases (although
not this one), Firefox uses event listeners for double click events, thus I
could use this method pretty much anywhere in the source without adapting it... 

It also seemed more elegant to me to take the argument and query it in the JS to
keep the XUL clean. In the end, it's not massively important, as long as it does
the job.

I think this could be better by underlying code, but that'll probably be a more
complex change. We can patch it with our simple fixes now and file a bug for a
more thorough fix later.

Comment 9

[Ben Basson]

Attachment: Left button only

Changed from blacklisting the right button to whitelisting the left button.

Comment 10

[Ben Basson]

Comment on attachment 150532 [details] [diff] [review]
Left button only

Sorry mconner, will need the review again. Just about to CVS diff and submit a
patch for the other bug too.

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 150532 [details] [diff] [review]
Left button only

mental note, coffee before reviews.

Thanks for catching this :(

Comment 12

[Jesse Ruderman]

This class of bugs should be fixed by making ondblclick only fire for double
left clicks, not by adding button checks everywhere the ondblclick event is used
in Firefox and Seamonkey UI.  This is a dup of bug 217560.

Comment 13

[Jesse Ruderman]

*** This bug has been marked as a duplicate of 217560 ***

Comment 14

[Steffen Wilberg]

Reopening per bug 217560 comment 13.
Blocks bug 244692 since the summary of that covers EM as well, but the patch for
EM is here.

Comment 15

[Steffen Wilberg]

Comment on attachment 150532 [details] [diff] [review]
Left button only

>Index: extensions.js
>-    cmd_options: function ()
>+    cmd_options: function (aEvent)
>     {
>+      if (aEvent.button != 0)
>+        return;
This is (bitrotten and) wrong. We need to do this in onViewDoubleClick(), which
calls cmd_options (EM) or cmd_useTheme (TM).

Comment 16

[Steffen Wilberg]

I'll attach a patch to bug 244692 which covers this as well.

*** This bug has been marked as a duplicate of 244692 ***