The default bug view has changed. See this FAQ.

Clicking any two mouse buttons opens new tab

VERIFIED FIXED in Firefox 6

Status

Firefox Graveyard
Panorama
VERIFIED FIXED
6 years ago
a year ago

People

(Reporter: George Carstoiu, Assigned: raymondlee)

Tracking

Trunk
Firefox 6
x86
All

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110411 Firefox/4.2a1pre

Clicking any two mouse buttons (left + right, left + middle etc) opens a new empty tab

Reproducible: always

Steps to reproduce:
 1. Enter Panorama
 2. Click left and right buttons in fast succession

Actual results: 
 - a new empty tab is created

Expected results:
 - nothing should happen considering that it's not the same button that is pressed twice
(Assignee)

Comment 1

6 years ago
Created attachment 525296 [details] [diff] [review]
v1
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Attachment #525296 - Flags: feedback?(tim.taubert)
Comment on attachment 525296 [details] [diff] [review]
v1

Sorry, but I think that is not the right place and way to fix this :)

1.) These are the lines that already fix the double click problem for other mouse buttons than the left (this concerns groupItems only).

>@@ -1670,26 +1671,29 @@ GroupItem.prototype = Utils.extend(new I
>     container.mousedown(function(e) {
>       if (!Utils.isLeftClick(e))
>         return;

2.) The same fix from (1) should go into "ui.js" somewhere at line 190+. That is the actual fix for this bug.
3.) As this bug points out with the fixes mentioned above we also consider the user made a double click when he quickly clicks left, right, left (for example). So we should make sure when the user does not click the left key we should reset all values like:

>@@ -1670,26 +1671,29 @@ GroupItem.prototype = Utils.extend(new I
>     container.mousedown(function(e) {
>      if (!Utils.isLeftClick(e) || self.$titlebar[0] == e.target ||
>          self.$titlebar.contains(e.target)) {
>         self._lastClick = 0;
>         self._lastClickPositions = null;
>         return;
>       }

4.) We need the check from (3) in ui.js, point (2), too.
5.) We should write tests for this :)
Attachment #525296 - Flags: feedback?(tim.taubert) → feedback-
(Assignee)

Comment 3

6 years ago
Created attachment 525651 [details] [diff] [review]
v2
Attachment #525296 - Attachment is obsolete: true
Attachment #525651 - Flags: feedback?(tim.taubert)

Updated

6 years ago
Blocks: 638188
Comment on attachment 525651 [details] [diff] [review]
v2

Thanks for the test cleanup! Nits:

>+  showTabView(function() {
>+    contentWindow = document.getElementById("tab-view").contentWindow;
>+    test1();
>+  });

Please use TabView.getContentWindow().

>+function test1() {
>+  is(gBrowser.tabs.length, 1, 
>+     "Total number of tabs is 1 before right button double click");
>+  // first click
>+  mouseClick(contentWindow.document.getElementById("content"), 2);
>+  // second click
>+  mouseClick(contentWindow.document.getElementById("content"), 2);

We should save the #content node in a variable. Same in test2(). We could also fetch that right after TabView.getContentWindow() to make it available to all test functions.

Looks good, f+ with these nits :)
Attachment #525651 - Flags: feedback?(tim.taubert) → feedback+
(In reply to comment #4)
> Please use TabView.getContentWindow().
> 
> We should save the #content node in a variable. Same in test2(). We could also
> fetch that right after TabView.getContentWindow() to make it available to all
> test functions.

Please apply those changes to browser_tabview_bug604098.js, too.
(Assignee)

Comment 6

6 years ago
Created attachment 526037 [details] [diff] [review]
v3

Updated the patch based on Tim's comments.
Attachment #525651 - Attachment is obsolete: true
Attachment #526037 - Flags: review?(ian)
(Assignee)

Comment 7

6 years ago
Comment on attachment 526037 [details] [diff] [review]
v3

Passed Try!
http://tbpl.mozilla.org/?tree=MozillaTry&rev=92a2758cceb3
Comment on attachment 526037 [details] [diff] [review]
v3

Looks good!
Attachment #526037 - Flags: review?(ian) → review+
(Assignee)

Comment 9

6 years ago
Created attachment 526381 [details] [diff] [review]
Patch for checkin
Attachment #526037 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/a2f846b209b5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 6
Version: 4.0 Branch → Trunk
(Reporter)

Comment 11

6 years ago
Verified on Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110421 Firefox/6.0a1
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.