Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Last Comment Bug 179845 - Support dragging a link onto the New Tab / New Window toolbar button.
: Support dragging a link onto the New Tab / New Window toolbar button.
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: unspecified
: All All
-- enhancement with 8 votes (vote)
: Firefox1.5
Assigned To: Ben Basson
:
: :Gijs
Mentors:
: 234203 240171 252880 269904 295068 (view as bug list)
Depends on: 477694
Blocks:
  Show dependency treegraph
 
Reported: 2002-11-12 16:28 PST by Nathan Silva
Modified: 2009-02-09 14:38 PST (History)
17 users (show)
bugzilla: blocking‑aviary1.0PR-
mconnor: blocking‑aviary1.5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v0.1 (7.30 KB, patch)
2004-07-07 06:54 PDT, Ben Basson
no flags Details | Diff | Splinter Review
Patch v0.1 - background tab (5.59 KB, patch)
2004-07-07 17:01 PDT, Ben Basson
no flags Details | Diff | Splinter Review
Patch v0.2 - background tab (6.09 KB, patch)
2004-08-14 12:50 PDT, Ben Basson
no flags Details | Diff | Splinter Review
Patch v0.3 (5.92 KB, patch)
2005-01-01 11:06 PST, Ben Basson
mconnor: review+
asa: approval‑aviary1.1a2+
Details | Diff | Splinter Review

Description User image Nathan Silva 2002-11-12 16:28:06 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021112 Phoenix/0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021112 Phoenix/0.4

You should be able to drag a link onto the New Tab toolbar button and have it
open in a tab.

Reproducible: Always

Steps to Reproduce:




Since you can drag a URL onto Home or Go you should be able to drag a link onto
the New Tab button.

This would be convenient if you use the pref "Hide the tab bar when only one tab
is open".
Comment 1 User image David P James 2002-11-17 17:20:11 PST
We're in danger of getting into feature bloat here, but since the other drag &
drop operations exist this one should be considered too.

-->Confirming as New for developer review
Comment 2 User image Simon Paquet [:sipaq] 2003-07-28 05:30:25 PDT
Taking QA Contact
Comment 3 User image Jesse Ruderman 2004-02-14 03:04:06 PST
*** Bug 234203 has been marked as a duplicate of this bug. ***
Comment 4 User image Kevin Ar18 2004-02-14 09:03:18 PST
Can we add "New Window" button to the subject as well?
Comment 5 User image Kevin Ar18 2004-02-16 17:42:07 PST
BTW, I think it would be appropriate if the buttons would appear "depressed"
whenever you hover over it with your mouse (and you have a URL you are dragging).
Comment 6 User image Asa Dotzler [:asa] 2004-05-04 17:33:10 PDT
*** Bug 240171 has been marked as a duplicate of this bug. ***
Comment 7 User image Ben Basson 2004-07-07 06:54:39 PDT
Created attachment 152515 [details] [diff] [review]
Patch v0.1

This should do the trick, however it could do with reviewing and, most likely,
tidying up. Can someone please point out to me a better way of opening an
active (selected) tab, if there is one?

I'm surprised that there was no function for doing so, or a flag for
openNewTabWith(), which would be the most logical way. I assume that would be a
seperate bug.

Anyway, this appears to work ok. I'll flag for review.
Comment 8 User image Ben Basson 2004-07-07 06:58:09 PDT
Comment on attachment 152515 [details] [diff] [review]
Patch v0.1

Can you check this out for me please Mike?
Comment 9 User image Ben Basson 2004-07-07 07:41:59 PDT
Nominating 1.0RC1 due to localisation impacts and the fact that it's exactly a
simple patch. Also security issues to consider. 

If it's going to miss 1.0, that's ok, but if someone wants to get this in, it
needs to be done before RC1. It's polish, but logical polish adding to the
precedent set by the Home and Go buttons.
Comment 10 User image Steffen Wilberg 2004-07-07 10:35:20 PDT
Why you don't use openNewTabWith()? That function checks the pref
"browser.tabs.loadInBackground" and the shift key to determine whether to select
the new tab. The same pref is checked when you drop a link on the tabbar, see
the onDrop method in tabbrowser.xml:
http://lxr.mozilla.org/aviarybranch/source/toolkit/content/widgets/tabbrowser.xml#1080

Dropping a link on the new tab button should have the same behaviour as dropping
on the tabbar: create a new tab, and select that tab if the pref is set to
false. Your patch always selects the new tab.
Comment 11 User image Ben Basson 2004-07-07 11:24:00 PDT
Clicking the "New Tab" button opens a focussed "about:blank" tab, ideally
dragging a link should perform the same action, in my opinion, which is why I
wrote my own version of openNewTabWith().
Comment 12 User image Steffen Wilberg 2004-07-07 11:53:38 PDT
The blank tab is selected so that you can enter an url in the urlbar. Opening a
blank tab in the background wouldn't make much sense.
But opening links in the background makes a lot of sense: you can open multiple
links from the same page. I do a middleclick for that, but there are people
without a third mousebutton; Mac users only have one.
Comment 13 User image Ben Basson 2004-07-07 17:01:29 PDT
Created attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

Same patch, but without changes to contentAreaUtils.js and tab opens in the
background. Whoever reviews this can decide.
Comment 14 User image Ben Basson 2004-07-07 17:02:24 PDT
Comment on attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

Here's another for you Mike... (don't kill me :D)
Comment 15 User image Blake Ross 2004-07-20 22:59:03 PDT
Not blocking, but we'll still take this patch.
Comment 16 User image Jesse Ruderman 2004-07-24 15:12:06 PDT
*** Bug 252880 has been marked as a duplicate of this bug. ***
Comment 17 User image Steffen Wilberg 2004-08-10 16:35:49 PDT
(In reply to bug 250974 comment #9)
> Steffen, sorry to bring this up here, but since it's related, any chance of you
> taking a look at my patch for Bug 179845 and getting it in? Apologies if I'm
> just being a pain!
The second patch looks good to me at a glance, but I'm not able to give reviews.
browser.properties has been moved to
mozilla/browser/locales/en-US/chrome/browser/browser.properties by the way.

-> cusser.
Comment 18 User image Ben Basson 2004-08-10 16:39:13 PDT
Will CVS take care of that or do I need to update the patch? If an update is
needed, I'll do it tomorrow afternoon.
Comment 19 User image Steffen Wilberg 2004-08-10 16:48:55 PDT
The person who wants to check in a patch first needs to patch his local tree by
using the patch command. That complains that the file doesn't exist, and one has
to enter it manually.

So it's not absolutely necessary for you to update the patch, but it's common
practice, and it helps, especially when time is running out.
And please run |cvs diff (file1) (file2) (file3)| from above mozilla/.
Comment 20 User image Asa Dotzler [:asa] 2004-08-11 21:52:19 PDT
Comment on attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

please don't request approval until you've got sufficient reviews.
Comment 21 User image Ben Basson 2004-08-14 12:50:56 PDT
Created attachment 156116 [details] [diff] [review]
Patch v0.2 - background tab

Thanks for your help Steffen, I've taken note of your comments, here's the
updated patch. Sorry asa, my bad. Will wait until this is reviewed, as
directed.
Comment 22 User image Steffen Wilberg 2004-08-15 03:33:44 PDT
Comment on attachment 152515 [details] [diff] [review]
Patch v0.1

Removing review request from obsolete patch.
Comment 23 User image Steffen Wilberg 2004-08-15 03:34:12 PDT
Comment on attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

Here as well.
Comment 24 User image Ben Basson 2004-09-07 04:40:17 PDT
Comment on attachment 156116 [details] [diff] [review]
Patch v0.2 - background tab

If you want this for 1.0, it needs to be reviewed and checked in before 1.0PR
due to localisation.

Since it's a simple patch, any chance of this happening?
Comment 25 User image David Baron :dbaron: ⌚️UTC-7 2004-09-12 14:38:13 PDT
Comment on attachment 156116 [details] [diff] [review]
Patch v0.2 - background tab

I'm not an appropriate reviewer for this patch.
Comment 26 User image Phil Ringnalda (:philor)(back in August) 2004-11-14 21:52:11 PST
*** Bug 269904 has been marked as a duplicate of this bug. ***
Comment 27 User image Simon Paquet [:sipaq] 2004-11-18 11:43:25 PST
Comment on attachment 156116 [details] [diff] [review]
Patch v0.2 - background tab

You don't need superreview for Firefox patches. You might try to ask Ben via
IRC to give a review, to get this in faster.
Comment 28 User image nrlz 2004-12-17 14:28:33 PST
I found some weird behavior with "Patch v0.2".

I can drag and drop links from within the browser onto those icons fine. But I
cannot drag and drop files from the desktop onto those icons. They won't open.

However, if I modify the order of the flavor sets, with text/unicode as the
first and x-moz-file as the last, then I can drag and drop desktop icons as
well. I don't understand why it works though but it just does. The new order
doesn't seem to impact the other features.

i.e.,

var flavourSet = new FlavourSet();
flavourSet.appendFlavour("text/unicode");
flavourSet.appendFlavour("text/x-moz-url");
flavourSet.appendFlavour("application/x-moz-file", "nsIFile");
return flavourSet;
Comment 29 User image Ben Basson 2005-01-01 11:06:42 PST
Created attachment 170039 [details] [diff] [review]
Patch v0.3

Patch v0.3 - Same as the last patch, with newer files (to prevent bitrot) and
reordering of the flavoursets to allow file drag/dropping. Also reordered
flavoursets for the Go button while I was there.
Comment 30 User image Ben Basson 2005-01-06 17:34:39 PST
Nominating to invoke review. It's not critical for 1.1, but it'd be nice to
finally land this and prevent future bitrot. 
Comment 31 User image Mike Connor [:mconnor] 2005-05-03 12:52:38 PDT
plussing to get on radar, we're getting pretty good with drag and drop support,
we can be better though.

Please stick this in my review queue, I will get to it this weekend/next week. 
Looks good, though I want to grok the flavourset changes better than my sick
head can do right now.
Comment 32 User image Ben Basson 2005-05-03 13:08:49 PDT
Comment on attachment 170039 [details] [diff] [review]
Patch v0.3

Done. Thanks Mike.
Comment 33 User image Tristor 2005-05-23 10:16:04 PDT
*** Bug 295068 has been marked as a duplicate of this bug. ***
Comment 34 User image Mike Connor [:mconnor] 2005-06-07 04:11:17 PDT
Comment on attachment 170039 [details] [diff] [review]
Patch v0.3

sometimes, prevailing style is evil... but I'd rather not reindent 5000+ lines
of JS :)

sorry about the delay, fixing my review queue now.
Comment 35 User image Ben Basson 2005-06-07 04:43:25 PDT
Comment on attachment 170039 [details] [diff] [review]
Patch v0.3

Thanks, Mike... and no worries :)

Requesting approval and checkin, please?
Comment 36 User image Mano (::mano, needinfo? for any questions; not reading general bugmail) 2005-06-07 12:43:58 PDT
Checking in base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.427; previous revision: 1.426
done
Checking in base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.262; previous revision: 1.261
done
Checking in locales/en-US/chrome/browser/browser.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.properties,v  <--
 browser.properties
new revision: 1.12; previous revision: 1.11
done
Comment 37 User image mv_van_rantwijk 2005-06-13 07:30:54 PDT
Why two observers? Why not one? Oh, and more importantly, what about that
security hole (javascript: and data: links) ?

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