Double click should be mouse button specific

VERIFIED DUPLICATE of bug 244692

Status

()

Toolkit
Add-ons Manager
--
minor
VERIFIED DUPLICATE of bug 244692
14 years ago
10 years ago

People

(Reporter: Ben Basson, Assigned: Ben Goodger (use ben at mozilla dot org for email))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

14 years ago
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.
(Reporter)

Comment 1

14 years ago
Created attachment 150493 [details] [diff] [review]
Check event button

Should solve this minor annoyance
(Reporter)

Updated

14 years ago
Attachment #150493 - Flags: review?

Comment 2

14 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Updated

14 years ago
Attachment #150493 - Flags: review? → review?(bugs)
(Reporter)

Comment 3

14 years ago
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 ;)

Updated

14 years ago
Flags: blocking1.0?
(Reporter)

Updated

14 years ago
Attachment #150493 - Flags: review?(bugs) → review?(mconnor)
Comment on attachment 150493 [details] [diff] [review]
Check event button

yeah, we need to do this is multiple places

r=mconnor@myrealbox.com
Attachment #150493 - Flags: review?(mconnor) → review+
This is awfully similar to bug 244692, which covers not only the extension
manager but also other GUI internals.
Depends on: 244692
(Reporter)

Comment 6

14 years ago
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

14 years ago
(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]
(Reporter)

Comment 8

14 years ago
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.
(Reporter)

Comment 9

14 years ago
Created attachment 150532 [details] [diff] [review]
Left button only

Changed from blacklisting the right button to whitelisting the left button.
Attachment #150493 - Attachment is obsolete: true
(Reporter)

Comment 10

14 years ago
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.
Attachment #150532 - Flags: review?(mconnor)
Comment on attachment 150532 [details] [diff] [review]
Left button only

mental note, coffee before reviews.

Thanks for catching this :(
Attachment #150532 - Flags: review?(mconnor) → review+

Comment 12

14 years ago
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

14 years ago

*** This bug has been marked as a duplicate of 217560 ***
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → DUPLICATE

Comment 14

14 years ago
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.
Blocks: 244692
Severity: trivial → minor
Status: RESOLVED → REOPENED
No longer depends on: 244692
Resolution: DUPLICATE → ---

Comment 15

14 years ago
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).
Attachment #150532 - Attachment is obsolete: true

Comment 16

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

*** This bug has been marked as a duplicate of 244692 ***
Status: REOPENED → RESOLVED
Last Resolved: 14 years ago14 years ago
Flags: blocking-aviary1.0?
Resolution: --- → DUPLICATE

Updated

14 years ago
No longer blocks: 244692

Updated

14 years ago
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.