Last Comment Bug 708640 - Should be able to open a new tab by middle-clicking the "Go" button.
: Should be able to open a new tab by middle-clicking the "Go" button.
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Location Bar (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: seamonkey2.8
Assigned To: Philip Chee
:
:
Mentors:
Depends on:
Blocks: 752336
  Show dependency treegraph
 
Reported: 2011-12-08 08:34 PST by Philip Chee
Modified: 2012-05-06 10:19 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Patch v1.0 checkForMiddleClick. (2.38 KB, patch)
2011-12-08 08:42 PST, Philip Chee
neil: review-
Details | Diff | Splinter Review
Patch v1.1 Use existing code block. (2.30 KB, patch)
2011-12-09 01:29 PST, Philip Chee
neil: review+
Details | Diff | Splinter Review
Patch v1.2 Patch as checked in. (2.29 KB, patch)
2011-12-10 02:43 PST, Philip Chee
philip.chee: review+
Details | Diff | Splinter Review

Description Philip Chee 2011-12-08 08:34:56 PST
Relevant Firefox bugs:
  [Bug 279687] Should be able to open new tab by middle-clicking "Go" button.
  [Bug 339710] Can't use "Go" button" with using keyword or space.
  [Bug 405541] Location Bar don't revert back to the correct url of the tab when you middle click go after entering text, to open in a new tab.
Comment 1 Philip Chee 2011-12-08 08:42:59 PST
Created attachment 580060 [details] [diff] [review]
Patch v1.0 checkForMiddleClick.

Is this the right approach?
Comment 2 neil@parkwaycc.co.uk 2011-12-08 09:52:29 PST
Does Ctrl+Clicking the "Go" button already work? I think that uses an earlier code block, might be worth seeing whether we can hook into that instead.
Comment 3 neil@parkwaycc.co.uk 2011-12-08 11:53:06 PST
Comment on attachment 580060 [details] [diff] [review]
Patch v1.0 checkForMiddleClick.

>+      var where = whereToOpenLink(aTriggeringEvent, false, false);
Indeed, this is wrong because it uses the wrong pref. Instead you need to look for a potential button 1 in the original conditional block.

>+                onclick="checkForMiddleClick(this, event);"
This part of the patch is fine of course.
Comment 4 Philip Chee 2011-12-09 01:29:28 PST
Created attachment 580347 [details] [diff] [review]
Patch v1.1 Use existing  code block.

> Does Ctrl+Clicking the "Go" button already work? I think that uses an earlier
> code block, might be worth seeing whether we can hook into that instead.
Fixed.

> Comment on attachment 580060 [details] [diff] [review]
> Patch v1.0 checkForMiddleClick.
> 
>>+      var where = whereToOpenLink(aTriggeringEvent, false, false);
> Indeed, this is wrong because it uses the wrong pref. Instead you need to look
> for a potential button 1 in the original conditional block.
Fixed.

>>+                onclick="checkForMiddleClick(this, event);"
> This part of the patch is fine of course.
Phew.
Comment 5 neil@parkwaycc.co.uk 2011-12-09 03:28:33 PST
Comment on attachment 580347 [details] [diff] [review]
Patch v1.1 Use existing  code block.

>         (('ctrlKey' in aTriggeringEvent && aTriggeringEvent.ctrlKey) ||
>-         ('metaKey' in aTriggeringEvent && aTriggeringEvent.metaKey))) {
>+         ('metaKey' in aTriggeringEvent && aTriggeringEvent.metaKey) ||
>+         (aTriggeringEvent instanceof MouseEvent && aTriggeringEvent.button == 1))) {
Would you mind using the same style as the other conditions?
Comment 6 Philip Chee 2011-12-10 02:43:45 PST
Created attachment 580630 [details] [diff] [review]
Patch v1.2 Patch as checked in.

>>         (('ctrlKey' in aTriggeringEvent && aTriggeringEvent.ctrlKey) ||
>>-         ('metaKey' in aTriggeringEvent && aTriggeringEvent.metaKey))) {
>>+         ('metaKey' in aTriggeringEvent && aTriggeringEvent.metaKey) ||
>>+         (aTriggeringEvent instanceof MouseEvent && aTriggeringEvent.button == 1))) {
> Would you mind using the same style as the other conditions?
Done.

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/3d2c908c2938

Note You need to log in before you can comment on or make changes to this bug.