Last Comment Bug 356005 - Middle click onto URL bar (address bar) icon opens a new tab
: Middle click onto URL bar (address bar) icon opens a new tab
Status: RESOLVED FIXED
: fixed-seamonkey1.1
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: 1.8 Branch
: x86 All
: -- minor with 1 vote (vote)
: ---
Assigned To: Benoît
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-10-09 02:04 PDT by Stefan A. Möller
Modified: 2008-07-31 04:23 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (794 bytes, patch)
2006-10-19 16:26 PDT, Benoît
csthomas: review-
Details | Diff | Review
revised and tested patch (1.13 KB, patch)
2006-11-13 17:08 PST, Benoît
csthomas: review+
neil: superreview+
csthomas: approval‑seamonkey1.1+
Details | Diff | Review

Description Stefan A. Möller 2006-10-09 02:04:15 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1b2) Gecko/20060821 SeaMonkey/1.1a
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1b2) Gecko/20060821 SeaMonkey/1.1a

A middle click onto the URL bar icon loads the clipboard content as a web address. In SeaMonkey 1.0 and Firefox (all Versions, including 2.0 Beta 2) the URL gets loaded in the current tab, whereas in Seamonkey 1.1a and 1.5a the behaviour has changed. Now a new tab is opened. To me, this seems to be a bug.

Reproducible: Always

Steps to Reproduce:
1. Copy a web address into the clipboard.
2. Middle click onto the icon inside the URL bar.


Actual Results:  
A new tab opens.


Expected Results:  
The current tab should be used.
Comment 1 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-10-10 16:56:32 PDT
http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/contentAreaClick.js#331
I'm guessing this check was not written with the consideration that you could middle-click the proxy icon.
Comment 2 neil@parkwaycc.co.uk 2006-10-11 02:06:14 PDT
Right... you'd either have to add a check that the target's binding parent (document.getBindingParent(tab)) is the tabbrowser, or add a parameter to middleMousePaste to indicate whether to check for a click on the tabbrowser.
Comment 3 Benoît 2006-10-19 16:26:21 PDT
Created attachment 242816 [details] [diff] [review]
patch

Made the change that Neil suggested.
Comment 4 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-10-19 17:08:14 PDT
Comment on attachment 242816 [details] [diff] [review]
patch

Sorry, this isn't right.  I can no longer middle click an empty portion of the tab strip and get a new tab now.  Neil, should he walk up the bindingparents instead of checking the immediate parent? (I assume bindingParent is ending up as button or tabs, but not tab strip).

BenoitRen, use more lines of context (diff -u9) for future patches.  It makes them more readable.
Comment 5 Stefan A. Möller 2006-10-20 02:55:47 PDT
> I can no longer middle click an empty portion of the tab strip and get
> a new tab now.

???

For me, that never worked in any Mozilla based Browser. (at least on Windows)
Comment 6 neil@parkwaycc.co.uk 2006-10-20 05:06:31 PDT
(In reply to comment #4)
>(From update of attachment 242816 [details] [diff] [review])
>Sorry, this isn't right.  I can no longer middle click an empty portion of the
>tab strip and get a new tab now.  Neil, should he walk up the bindingparents
>instead of checking the immediate parent? (I assume bindingParent is ending up
>as button or tabs, but not tab strip).
Actually I think the binding parent of the blank spacer on the tab strip is the tab strip ;-) OK, so my next idea is to check (event.target == gBrowser).
Comment 7 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-10-20 06:05:27 PDT
(In reply to comment #5)
> > I can no longer middle click an empty portion of the tab strip and get
> > a new tab now.
> 
> ???
> 
> For me, that never worked in any Mozilla based Browser. (at least on Windows)
> 

Right, I was testing with contentLoadURL and middlemouse.paste set, to make sure it wouldn't break linux.
Comment 8 Benoît 2006-11-13 17:08:20 PST
Created attachment 245512 [details] [diff] [review]
revised and tested patch
Comment 9 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-11-15 15:53:37 PST
Comment on attachment 245512 [details] [diff] [review]
revised and tested patch

r=me.  Remember that this needs to be applied to both xpfe and suite versions of the file when you check it in.
Comment 10 Benoît 2006-11-28 10:41:03 PST
Comment on attachment 245512 [details] [diff] [review]
revised and tested patch

Patch has baken on trunk since 17/11. Requesting approval for branch landing.
Comment 11 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2006-11-28 15:41:52 PST
Comment on attachment 245512 [details] [diff] [review]
revised and tested patch

a=me for 1.1 (this is simple enough that I don't think it requires second-a)
Comment 12 Andrew Schultz 2006-11-29 19:31:01 PST
landed on branch

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