Support dragging a link onto the New Tab / New Window toolbar button.

RESOLVED FIXED in Firefox1.5

Status

()

--
enhancement
RESOLVED FIXED
17 years ago
10 years ago

People

(Reporter: nathans, Assigned: bugzilla)

Tracking

unspecified
Firefox1.5
Points:
---
Bug Flags:
blocking-aviary1.0PR -
blocking-aviary1.5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

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

17 years ago
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Taking QA Contact
QA Contact: asa → bugzilla

Comment 3

15 years ago
*** Bug 234203 has been marked as a duplicate of this bug. ***

Comment 4

15 years ago
Can we add "New Window" button to the subject as well?

Updated

15 years ago
OS: Windows XP → All
Hardware: PC → All

Comment 5

15 years ago
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).
Summary: Support dragging a link onto the "New Tab" toolbar button. → Support dragging a link onto the New Tab / New Window toolbar button.

Comment 6

15 years ago
*** Bug 240171 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

15 years ago
Posted patch Patch v0.1 (obsolete) — Splinter Review
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.
(Assignee)

Comment 8

15 years ago
Comment on attachment 152515 [details] [diff] [review]
Patch v0.1

Can you check this out for me please Mike?
Attachment #152515 - Flags: review?(mconnor)
(Assignee)

Comment 9

15 years ago
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.
Flags: blocking-aviary1.0RC1?

Comment 10

15 years ago
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.
(Assignee)

Comment 11

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

15 years ago
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.
(Assignee)

Comment 13

15 years ago
Posted patch Patch v0.1 - background tab (obsolete) — Splinter Review
Same patch, but without changes to contentAreaUtils.js and tab opens in the
background. Whoever reviews this can decide.
(Assignee)

Comment 14

15 years ago
Comment on attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

Here's another for you Mike... (don't kill me :D)
Attachment #152561 - Flags: review?(mconnor)

Comment 15

15 years ago
Not blocking, but we'll still take this patch.
Flags: blocking-aviary1.0RC1? → blocking-aviary1.0RC1-

Comment 16

15 years ago
*** Bug 252880 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

15 years ago
Attachment #152561 - Flags: approval-aviary?

Comment 17

15 years ago
(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.
Assignee: hyatt → cusser
(Assignee)

Comment 18

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

15 years ago
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 on attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

please don't request approval until you've got sufficient reviews.
Attachment #152561 - Flags: approval-aviary?
(Assignee)

Comment 21

15 years ago
Posted patch Patch v0.2 - background tab (obsolete) — Splinter Review
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.
Attachment #152561 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #152515 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #156116 - Flags: review?(firefox)

Comment 22

15 years ago
Comment on attachment 152515 [details] [diff] [review]
Patch v0.1

Removing review request from obsolete patch.
Attachment #152515 - Flags: review?(mconnor)

Comment 23

15 years ago
Comment on attachment 152561 [details] [diff] [review]
Patch v0.1 - background tab

Here as well.
Attachment #152561 - Flags: review?(mconnor)
(Assignee)

Comment 24

15 years ago
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?
Attachment #156116 - Flags: superreview?(bugs)
Attachment #156116 - Flags: review?(firefox)
Attachment #156116 - Flags: review?(dbaron)
Comment on attachment 156116 [details] [diff] [review]
Patch v0.2 - background tab

I'm not an appropriate reviewer for this patch.
Attachment #156116 - Flags: review?(dbaron) → review?
(Assignee)

Updated

15 years ago
Attachment #156116 - Flags: review? → review?(bugs)
*** Bug 269904 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
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.
Attachment #156116 - Flags: superreview?(bugs)

Comment 28

15 years ago
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;
(Assignee)

Updated

14 years ago
Attachment #156116 - Flags: review?(bugs)
(Assignee)

Comment 29

14 years ago
Posted patch Patch v0.3Splinter Review
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.
Attachment #156116 - Attachment is obsolete: true
Attachment #170039 - Flags: review?(bugs)
(Assignee)

Comment 30

14 years ago
Nominating to invoke review. It's not critical for 1.1, but it'd be nice to
finally land this and prevent future bitrot. 
Flags: blocking-aviary1.1?
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.
Flags: blocking-aviary1.1? → blocking-aviary1.1+
(Assignee)

Comment 32

14 years ago
Comment on attachment 170039 [details] [diff] [review]
Patch v0.3

Done. Thanks Mike.
Attachment #170039 - Flags: review?(bugs) → review?(mconnor)

Comment 33

14 years ago
*** Bug 295068 has been marked as a duplicate of this bug. ***
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.
Attachment #170039 - Flags: review?(mconnor) → review+
(Assignee)

Comment 35

14 years ago
Comment on attachment 170039 [details] [diff] [review]
Patch v0.3

Thanks, Mike... and no worries :)

Requesting approval and checkin, please?
Attachment #170039 - Flags: approval-aviary1.1a2?

Updated

14 years ago
Attachment #170039 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
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
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.1
Depends on: 297005
(Assignee)

Updated

14 years ago
No longer depends on: 297005

Comment 37

14 years ago
Why two observers? Why not one? Oh, and more importantly, what about that
security hole (javascript: and data: links) ?
QA Contact: bugzilla → toolbars
Depends on: 477694
You need to log in before you can comment on or make changes to this bug.