Closed Bug 225680 Opened 21 years ago Closed 16 years ago

Ability to detach/tear off tabs (Open a tab in a new window)

Categories

(Firefox :: Tabbed Browser, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: Marcin.Kasperski, Assigned: asaf)

References

(Depends on 2 open bugs, Blocks 3 open bugs)

Details

(Keywords: verified1.9.1, Whiteboard: [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera])

Attachments

(2 files, 22 obsolete files)

20.33 KB, patch
Details | Diff | Splinter Review
18.14 KB, patch
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031007 Firebird/0.7
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031007 Firebird/0.7

Galeon possesses very nice 'detach tab' option (Right-Click tab, option 'Detach
Tab' in menu). After this menu is selected, the new browser window is opened
containing the page which previously belonged to that tab.

Would be nice to be seen in firebird...

Reproducible: Always

Steps to Reproduce:
this is firebird version of Bug 102132
Summary: Option to detach the tab → Ability to detach tabs
Blocks: 126299
I quick search on "tab" didn't turn up any duplicates.
Status: UNCONFIRMED → NEW
Ever confirmed: true
*** Bug 228239 has been marked as a duplicate of this bug. ***
As I mentioned in my dup request 229239, it'd be nice if the tabs could be torn
off as well. In fact, the ultimate would be to drag and drop it onto the pager
applet, but that might be a real pipedream.
*** Bug 234304 has been marked as a duplicate of this bug. ***
*** Bug 268329 has been marked as a duplicate of this bug. ***
QA Contact: firefox.general
Summary: Ability to detach tabs → Ability to detach tabs (Open a tab in a new window)
Assignee: firefox → bugs
Component: General → Tabbed Browser
QA Contact: firefox.general → firefox.tabbed-browser
It would be nice to detach tabs, and even move them around, much like galeon
allows.  (example: move the rightmost tab to the left.)
Summary: Ability to detach tabs (Open a tab in a new window) → Ability to detach/tear off tabs (Open a tab in a new window)
*** Bug 293675 has been marked as a duplicate of this bug. ***
It would also be nice if the implementation ( if/whenever it comes ) uses the
already rendered page, does not reload it. I have noticed this as a possible
shortcomming - Konqueror reloads the page.
*** Bug 302413 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary2.0?
I don't think there should be a 'detach tab' option, but more likely ability to
drag tabs out of the window to turn them into windows of their own.
Of cource this combines with ability to drag tabs around in the window, and drag
from one window to another.
I definitely vote for the tear off UI. Drag across the tab bar to place, drag
off the tabbar to tear off, and if one is feeling ambitious, drag to someplace
(a trashcan icon? The trashcan icon on the users desktop?) to kill the tab.

Also, drag to bookmarks to save.

In general, make a dragging behavior, with some way of making it very easy to
specify what happens where.
If support for tearing tabs out of windows is written, I think that a "Close
Tab" option should be added to the right click list with "New Tab, Reload Tab,
Reload All Tabs, Close Other Tabs, Close Tab".
In my opinion 'Detach Tab' option in context menu would be nice too. Dragging 
is also nice but not always one have some free space to drag to, it is also 
very unpleasant operation to perform using touchpad so should never be the only 
way to do some thing. 
This requires Gecko changes that won't happen on the 1.8 branch for Fx2
Flags: blocking-aviary2? → blocking-aviary2-
Assignee: bugs → nobody
What Gecko changes does this depend on?  Does it depend on bug 254144?
Does the relatively new branch code allowing for tab dragging make this any easier?  It's been around for a while now on both 1.8.0 and 1.8 and it would be great not to have to wait for Firefox 3 to come out!

Seems that if we can now drag a tab, it's just a matter of checking where it's dropped and making a new window if needed.  But I don't really know what I'm talking about, it's maybe very difficult!
*** Bug 359839 has been marked as a duplicate of this bug. ***
the tearing-off motif does not work for users with maximized browser windows - there's nowhere to drag it!

this bug has been reported eight times since 2003. i'm shocked it hasn't got more attention.
(In reply to comment #20)
> the tearing-off motif does not work for users with maximized browser windows -
> there's nowhere to drag it!

/me has two montiors ;)
> the tearing-off motif does not work for users with maximized browser windows -
> there's nowhere to drag it!

In Adium, if you drag a tab above or below the tab bar, it immediately appears as a separate window.  Drag it back (as part of the same drag motion or even later), and it re-attaches.  Not sure how hard that is to implement on Windows ;)
I believe the discussion is going towards the less important things.
The crucial subject of this issue is to have an opportunity to detach the tab. SOMEHOW. But with protecting url, and the page content, and without reloading it.

In my opinion, it would be best to have 3 ways of doing this: via right-click menu on the tab, via drag outside the window, and via keyboard shortcut (Ctrl-something). In my opinion, drag is the least useful, and possibly confusing (as one can also expect that drag operation will create link to the page on the desktop for instance). But the main thing is to have the feature at all...
i agree with marcin that there should be multiple vectors to make it happen. different users will find each one variously useful. is there any argument against providing the option, though? the tab context menu already has 7 options and 3 separators.
(half of those should not be there, BTW, as things like New Tab, Reload All Tabs, or Bookmark All Tabs does not seem to be operations on this tab. But this is another story.)
(In reply to comment #23)
> one can also expect that drag operation will create link to the
> page on the desktop for instance

The Windows convention for this would be Ctrl+Shift+Drag.  Also, Right-Drag normally opens a context menu showing Copy, Move, Shortcut, etc.
In FF 2, you can do something like this in 2 steps: Ctl-N to open a new window, then drag the tab into the new window.   It would be really convenient if this could be reduced to just one step.  I vote for at least adding "Open in new window" to the right-click tab context menu.
> In FF 2, you can do something like this in 2 steps: Ctl-N to open a new window, then drag the tab into the new window. It would be really convenient if this could be reduced to just one step.

This way everything that has been changed in the DOM gets lost, which makes it quite useless (speaking for myself ofcourse).
Would adding "Open Tab in New Window" to the tab context menu also destroy the DOM changes?
Not necessarilly, but SHEER [re-reading of the document into a new window] would.

Following sentence IMO could easily get understood like that "loading the same URL _once again_ in a new window upon single click" is the goal:
> It would be really convenient if this could be reduced to just one step.

I just want to emphasis this one: Preserve the DOM please!
Priority: -- → P2
Whiteboard: PRD:TAB-003a parity-safari
Blocks: 403441
I hate to be the guy who pours cold water on this. But...

It could be possible that this would edge us close to Adobe's patents on GUI tabs. In my strictly non-legal understanding, this relates to transferring tabs between different windows (and probably, by extension, new windows). And yeah, I know, software patents, prior art, we're all for the public good, etc.

http://en.wikipedia.org/wiki/Tab_(GUI)#Patent_dispute

So I guess the question is: do we need legal counsel, or are the chances of tripping this patent so remote that we can just plough on?
That's(In reply to comment #34)
> I hate to be the guy who pours cold water on this. But...
> 
> It could be possible that this would edge us close to Adobe's patents on GUI
> tabs. In my strictly non-legal understanding, this relates to transferring tabs
> between different windows (and probably, by extension, new windows). And yeah,
> I know, software patents, prior art, we're all for the public good, etc.
> 
> http://en.wikipedia.org/wiki/Tab_(GUI)#Patent_dispute
> 
> So I guess the question is: do we need legal counsel, or are the chances of
> tripping this patent so remote that we can just plough on?

That's an interesting point, to which all I can say is that you can tear-away tabs in Safari already.  Of course, no doubt Apple has quite a bit of legal counsel.
(In reply to comment #35)
> That's(In reply to comment #34)
> > I hate to be the guy who pours cold water on this. But...
> > 
> > It could be possible that this would edge us close to Adobe's patents on GUI
> > tabs. In my strictly non-legal understanding, this relates to transferring tabs
> > between different windows (and probably, by extension, new windows). And yeah,
> > I know, software patents, prior art, we're all for the public good, etc.
> > 
> > http://en.wikipedia.org/wiki/Tab_(GUI)#Patent_dispute
> > 
> > So I guess the question is: do we need legal counsel, or are the chances of
> > tripping this patent so remote that we can just plough on?
> 
> That's an interesting point, to which all I can say is that you can tear-away
> tabs in Safari already.  Of course, no doubt Apple has quite a bit of legal
> counsel.
> 

Are you saying that since Apple's legal counsel has determined that they're not liable, that Mozilla doesn't need to seek counsel on the matter?

Personally, I would question the geographic jurisdiction of Abobe's patent. Should users in some parts of the world be deprived of such a useful feature just because of patent issues in other parts of the world?
Opera 9.27 has "floating panes".  This allows one to cascade or tile (like excel, etc) tabbed panes in the main window.  Having floating panes also allows one to "detach tab" or drag the tab off the main window to the desktop.  This floating pane has no menu bar.  Other tool bars can be set as the user desires and then it remembers those settings as well as the size to which you set the first detached tab.

Safari nor Opera 9.5Beta, don't "detach tab".  They just create new windows.

<alterna> in the Mozilla forum gave me the script:

Left: javascript:%20resizeTo(screen.availWidth/2,screen.availHeight);window.moveTo(0,0); 

which duplicates the behavior of Safari and Opera 9.5.  (without the drag part)

In the Opera side panel is a "window" function that allow fine grained control of windows vs tabs and where they are located.

Anyway, the Opera 9.27 feature is what I'd like to see.  It is extremely useful for research, docs, faqs, help pages, videos, etc.  

As a tab, it doesn't have a menu bar, it can just function like a pop up window that the USER creates to suit his (or her) needs.


Great feature for FF3.1!  hint hint ;-)




   
Depends on: 448546
Depends on: 113934
No longer depends on: 448546
What patch does is:
Ctrl+Shift+C - to duplicate a tab (from clone)
Ctrl+Shift+N - to duplicate a tab in a new window
Ctrl+Shift+M - to move a tab to a new window(detach tab)
The above options are also added to the tab context menu(Does that menu get to many options in this way?).
Detach tab is implemented using swapBrowsersAndCloseOther fom bug 113934, so that needs to be checked in first.
For duplicating a tab I'm using the same duplicateTabIn function mentioned in bug 448546(but with the changes Dão Gottwald asked me to do).
Attachment #332731 - Flags: ui-review?(beltzner)
Attachment #332731 - Flags: review?(dao)
I didn't implement the drag to desktop detach tab behavior, because that is already used for saving a shortcut of the url from the dragged tab to the desktop.
Comment on attachment 332731 [details] [diff] [review]
patch - Use swapBrowsersAndCloseOther from bug 113934

>+                          oncommand="var tabbrowser = this.parentNode.parentNode.parentNode.parentNode;
>+                                     tabbrowser.duplicateTabInTab(tabbrowser.mContextTab);"/>

Nit: I think it's better to use gBrowser here since the tabbrowser binding isn't in toolkit anymore.

I don't think we want to clutter the context menu like this. Please re-request review once the ui-review is done.
Attachment #332731 - Flags: review?(dao)
I'm now using Clone Tab so the Ctrl+Shift+C makes sense(Ctrl+Shift+D is already in use). So where the screenshot currently says "Duplicate", it now should be "Clone". Besides I've also addressed the comments from Dão.
Attachment #332731 - Attachment is obsolete: true
Attachment #332878 - Flags: ui-review?(beltzner)
Attachment #332731 - Flags: ui-review?(beltzner)
Attachment #332878 - Attachment is obsolete: true
Attachment #332952 - Flags: ui-review?(beltzner)
Attachment #332878 - Flags: ui-review?(beltzner)
Attachment #332736 - Attachment is obsolete: true
Attachment #332953 - Flags: ui-review?(beltzner)
Attachment #332736 - Flags: ui-review?(beltzner)
(In reply to comment #45)
> Created an attachment (id=332953) [details]

Note: you can't use "N" as an accesskey for "Clone Tab To New Window" item, it is already used for New Tab.
(In reply to comment #46)
> (In reply to comment #45)
> > Created an attachment (id=332953) [details] [details]
> 
> Note: you can't use "N" as an accesskey for "Clone Tab To New Window" item, it
> is already used for New Tab.

Thanks for catching this, I'm now using "W" as an acceskey.
Attachment #332952 - Attachment is obsolete: true
Attachment #332963 - Flags: ui-review?(beltzner)
Attachment #332952 - Flags: ui-review?(beltzner)
Attached image Screenshot with patch v4 applied (obsolete) —
Attachment #332953 - Attachment is obsolete: true
Attachment #332966 - Flags: ui-review?(beltzner)
Attachment #332953 - Flags: ui-review?(beltzner)
(In reply to comment #48)
> Created an attachment (id=332966) [details]
> Screenshot with patch v4 applied
> 

"Undo close tab" is missing from that screenshot. 

This fix makes the context menu of tabs the biggest one compared to other browsers.
(In reply to comment #49)
> (In reply to comment #48)
> > Created an attachment (id=332966) [details] [details]
> > Screenshot with patch v4 applied
> > 
> 
> "Undo close tab" is missing from that screenshot. 
The screenshot was made of a window in which there weren't any tabs to unclose, when this is the case "Undo Close Tabs" disappears automatically.

> This fix makes the context menu of tabs the biggest one compared to other
> browsers.
The menu becomes very big I agree, maybe I could move these options to a "Duplicate" submenu in that menu. Also I think "Reload Other Tabs" and "Bookmark All Tabs" (and maybe even "Undo Close Tab"), don't have anything to do with the current right-clicked tab. So maybe some of those could be removed and only be shown when right-clicking an empty part of the tabbar, and not when right-clicking on a tab.
I've remove "Clone Tab To New Window" from the tab menu, which gives the tab menu a total of 10 items, that is the same amount of items Opera has in that menu (although it's still allot).
Attachment #332963 - Attachment is obsolete: true
Attachment #333581 - Flags: ui-review?(beltzner)
Attachment #332963 - Flags: ui-review?(beltzner)
Attached image Screenshot with patch v5 applied (obsolete) —
The behavior from "Clone tab to New Window" can still be achieved by first clicking Duplicate Tab and after that Move Tab To New Window. By removing "Clone Tab To New Window" the tab menu gets a little less big.
Attachment #332966 - Attachment is obsolete: true
Attachment #333582 - Flags: ui-review?(beltzner)
Attachment #332966 - Flags: ui-review?(beltzner)
The part that made it possible to go to an index is removed from this patch because it doesn't really belong to this bug (it should be in bug 448546). The code I've removed will be moved to a patch in bug 448546.
Attachment #333581 - Attachment is obsolete: true
Attachment #334732 - Flags: ui-review?(beltzner)
Attachment #333581 - Flags: ui-review?(beltzner)
Blocks: 448546
Because it is not entirely clear what the best way is to implement the features from this bug i started a discussion in moz.dev.apps.firefox, see:
http://groups.google.com/group/mozilla.dev.apps.firefox/browse_thread/thread/701b4f54f135573d#
It would be good to refactor the swap-and-close code instead of duplicating it from the drag/drop handler...
(In reply to comment #55)
> It would be good to refactor the swap-and-close code instead of duplicating it
> from the drag/drop handler...

Do you mean moving this stuff to swapBrowsersAndCloseOther?
> var newBrowser = this.getBrowserForTab(newTab);
> // Stop the about:blank load
> newBrowser.stop();
> // make sure it has a docshell
> newBrowser.docShell;
>              
> this.moveTabTo(newTab, newIndex);             
> ...
> // We need to set selectedTab after we've done
> // swapBrowsersAndCloseOther, so that the updateCurrentBrowser
> // it triggers will correctly update our URL bar.
> ...
> this.setTabTitle(newTab);
(In reply to comment #55)
> It would be good to refactor the swap-and-close code instead of duplicating it
> from the drag/drop handler...

Boris Zbarsky, is this what you meant by comment 55?
Attachment #334732 - Attachment is obsolete: true
Attachment #334985 - Flags: ui-review?(beltzner)
Attachment #334732 - Flags: ui-review?(beltzner)
Either that or having another method that does that and swaps.  I'd check with gavin on which he prefers, probably.
Boris Zbarsky, when detaching a tab, the new window that gets created is automatically opened with a new "about:blank tab", so you would expect I could use "var newTab = win.mTabs[0];" instead of "var newTab = win.addTab("about:blank");". But when I do that toolbar items (back/forward, reload and stop buttons) aren't synchronized(they're all grayed out). What is the reason I can only use addTab together with swapBrowsersAndCloseOther?

This is what I mean but doesn't work well:
> var newTab = win.mTabs[0];// which is in fact a "about:blank" tab
> win.swapBrowsersAndCloseOther(newTab, aTab);
I have no idea.  You'd have to ask someone who's actually familiar with this code...
(In reply to comment #60)
> I have no idea.  You'd have to ask someone who's actually familiar with this
> code...

Ok thanks, I thought maybe you knew because you wrote the patch in bug 113934. Gavin Sharp, have you got any ideas about comment 59?
Comment on attachment 333582 [details]
Screenshot with patch v5 applied

I think these are options which aren't really that commonly required, and in the case of Move Tab To New Window, better accomplished by drag and drop.
Attachment #333582 - Flags: ui-review?(beltzner) → ui-review-
This patch doesn't add the commandkeys and the tab menuitems anymore, instead you can duplicate/detach a tab in the following ways:
* Drag the tab to the content area of the browser - to detach the tab.
* Drag the tab to the content area of the browser while holding ctrl/cmd - to clone the tab to a new window.
* Drag the tab over the new tab button - to clone the tab to the last position on the tabbar.
* Drag the tab over the new window button - to clone the tab to a new window.

Dragging the tab to the desktop isn't (yet) implemented because I couldn't find the code that makes this possible (I think this is somewhere in C++). In the discussion I started there are a lot of comments from people who never use the the desktop (or are working with a maximized browser window), so I'm not sure how bad it is that the drag to desktop part doesn't work.
Attachment #333582 - Attachment is obsolete: true
Attachment #334985 - Attachment is obsolete: true
Attachment #335349 - Flags: ui-review?(beltzner)
Attachment #334985 - Flags: ui-review?(beltzner)
Some stuff that isn't needed anymore is removed now that there aren't any commandkeys and menuitems. Besides that I've also added the possibility to duplicate the new tab selected by dragging over the new tab button by holding shift.

Detaching a tab by dragging it to the desktop still isn't implemented in this patch. 

This is how the patch works now:
* Drag the tab to the content area of the browser - to detach the tab.
* Drag the tab to the content area of the browser while holding ctrl/cmd - to
clone the tab to a new window.
* Drag the tab over the new tab button - to clone the tab to the last position
on the tabbar, in background(depending on the pref).
* Drag the tab over the new tab button while holding shift - to clone the tab to the last position
on the tabbar, selected(depending on the pref).
* Drag the tab over the new window button - to clone the tab to a new window.
Attachment #335349 - Attachment is obsolete: true
Attachment #335695 - Flags: ui-review?(beltzner)
Attachment #335349 - Flags: ui-review?(beltzner)
This removes some code from patch v9 which isn't needed. comment 64 still applies to this patch.
Attachment #335695 - Attachment is obsolete: true
Attachment #335696 - Flags: ui-review?(beltzner)
Attachment #335695 - Flags: ui-review?(beltzner)
The clone tab menu option is added back because I think it makes duplicating a tab much easier (and more discoverable). Beta 2 of IE8 also adds a duplicate tab to the tab menu, so that's shows that it is a common feature.

This is how the patch works now:
* Drag the tab to the content area of the browser - to detach the tab.
* Drag the tab to the content area of the browser while holding ctrl/cmd - to
clone the tab to a new window.
* Drag the tab over the new tab button - to clone the tab to the last position
on the tabbar, in background(depending on the pref).
* Drag the tab over the new tab button while holding shift - to clone the tab
to the last position
on the tabbar, selected(depending on the pref).
* Drag the tab over the new window button - to clone the tab to a new window.
* A clone tab menu option is added to the tab context menu.

Mike Beltzner, do you think detaching a tab by dragging it to the desktop is something that is really needed for this.
Attachment #335696 - Attachment is obsolete: true
Attachment #335827 - Flags: ui-review?(beltzner)
Attachment #335696 - Flags: ui-review?(beltzner)
Attached image screenshot with the latest patch applied (obsolete) —
The patch I submitted should ofcourse be v11 instead of v10 (there are two v10s now). This screenshot shows the new "clone tab" menuitem plus the commandkey(IE8 is using Ctrl+K for that but I'm not sure why)
Attachment #335829 - Flags: ui-review?(beltzner)
Attachment #335827 - Attachment description: patch v10 - add a clone tab menu item → patch v11 - add a clone tab menu item
Attachment #335827 - Flags: review?(dao)
Dão, can you review the detachTab method and the duplicateTabIn function from this patch? Mike Beltzner is positive about detaching/cloning of tabs by drag and drop, he hasn't yet looked at the clone tab menuitem from the latest screenshot. So what I am asking is if you could review this patch without the menuitem part?
My feedback as somebody who's been wanting this for a while... (I haven't looked at the code, just going on what's been said)

(In reply to comment #66)
> * Drag the tab over the new tab button - to clone the tab to the last position
> on the tabbar, in background(depending on the pref).
> * Drag the tab over the new tab button while holding shift - to clone the tab
> to the last position
> on the tabbar, selected(depending on the pref).

What's the real difference here. If you have a pref, why use a modifier? (and vice versa). Again, I didn't look at the code, so maybe this has an obvious answer.

> Mike Beltzner, do you think detaching a tab by dragging it to the desktop is
> something that is really needed for this.

I'm not Mike, but I would say most definitely... and I would go further and say that just dragging anywhere outside the browser window should detach. Making the document the only target for detaching seems too specific. Casual users (assuming they are using this feature) likely won't think to drag just to the DOM. Though, this could lead to some confusion since current behavior of dragging to desktop creates a shortcut...

Play around with Safari if you haven't already. I really like how they do it with the accompanying animation/image to illustrate it becoming a new window. Not that we have to do it that way, but it is pretty sexy.
(In reply to comment #69)
> (In reply to comment #66)
> > ...
> 
> What's the real difference here. If you have a pref, why use a modifier? (and
> vice versa). Again, I didn't look at the code, so maybe this has an obvious
> answer.

Actually it already works like that, but without retaining history. You can use the modifier to reverse the behavior selected with the pref, so Shift+Drag would load it in the background while just dragging it to the new tab button would load the tab selected (or vice versa if loadInBackground is true).

> > ...
> 
> I'm not Mike, but I would say most definitely... and I would go further and
> say that just dragging anywhere outside the browser window should detach.
> Making the document the only target for detaching seems too specific. Casual
> users (assuming they are using this feature) likely won't think to drag just
> to the DOM. Though, this could lead to some confusion since current behavior
> of dragging to desktop creates a shortcut...

This is exactly how I intended to implement it, but the problem is that the code that handles this is I think in C++. That makes it much harder to implement it in that way. Maybe there could be filed a followup bug to make it work like that.

> Play around with Safari if you haven't already. I really like how they do it
> with the accompanying animation/image to illustrate it becoming a new window.
> Not that we have to do it that way, but it is pretty sexy.

Yeah, I looked at every major browser (IE8b2, Opera 9.5 and Safari 3.1) for inspiration, but again I think detaching by dragging anywhere outside the window is something for a followup (also because the create shortcut by dragging to desktop behavior needs to be disabled then).
This feature is something present in Chrome, and was mentioned by a friend of mine as the one Chrome feature that he REALLY wishes Firefox had.  The particular reason my friend really wants the feature is because he often finds himself needing to look at two pages side-by-side, which one can't do with tabs in the same window.

For what it's worth, it's also one of the features that I simply *assumed* Firefox had, but was surprised to discover otherwise.
(In reply to comment #71)
> This feature is something present in Chrome, and was mentioned by a friend of
> mine as the one Chrome feature that he REALLY wishes Firefox had.

I agree with Atul... this feature was important before and Chrome's excellent implementation of it makes it moreso.  Chrome has a feel of tabs being light and modifiable - objects you throw around and regroup at will.  I think this is more the feel we should strive for, and detaching with DOM state is a positive step forward.
Attached patch patch v12 - some small fixes (obsolete) — Splinter Review
This fixes some small issues from the latest patch. Comment #66 and the latest screenshot still apply to this patch.
Assignee: nobody → klaas1988
Attachment #335827 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #337351 - Flags: ui-review?(beltzner)
Attachment #337351 - Flags: review?(dao)
Attachment #335827 - Flags: ui-review?(beltzner)
Attachment #335827 - Flags: review?(dao)
Keywords: uiwanted
Whiteboard: PRD:TAB-003a parity-safari → [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] [has patch] [needs review beltzner]
(In reply to comment #66)
> * Drag the tab to the content area of the browser - to detach the tab.

I think this is too limiting. We should be modifying things so that when dragging a tab:

 - if drag target is along tabstrip of any window (+/-20px of height and width as a buffer) then the tab should be moved to that point in that window's tabstrip 
 - else, we should be spawning a new window

> * Drag the tab to the content area of the browser while holding ctrl/cmd - to
> clone the tab to a new window.

The shortcut for cloning in OSX is actually "option", so we should use that. On Windows ctrl is correct. The drag targetting should work as above.

> * Drag the tab over the new tab button - to clone the tab to the last position
> on the tabbar, in background(depending on the pref).
> * Drag the tab over the new tab button while holding shift - to clone the tab
> to the last position on the tabbar, selected(depending on the pref).

Foreground/background preferences aren't needed here and just muddy things up, IMO. I can't really see a good reason for dragging a tab and then leaving it in the background.

To duplicate within a tab strip, a opt/ctrl-drag with a target on the tabstrip should suffice. No need to target some small new tab button (which, I'll point out, is purely hypothetical at this point :)

> * Drag the tab over the new window button - to clone the tab to a new window.

New window button? No way.

> * A clone tab menu option is added to the tab context menu.

Sigh, I suppose "duplicate tab" might have merit. That menu is getting awful long, though. :(

> Mike Beltzner, do you think detaching a tab by dragging it to the desktop is
> something that is really needed for this.

Absolutely, 100%, yes.

(In reply to comment #70)
> This is exactly how I intended to implement it, but the problem is that the
> code that handles this is I think in C++. That makes it much harder to
> implement it in that way. Maybe there could be filed a followup bug to make it
> work like that.

That's fine, and Jim Mathies (cc'd on this bug already) has said that he'd be interested in helping out. Jim: what's needed here?

> window is something for a followup (also because the create shortcut by
> dragging to desktop behavior needs to be disabled then).

Not as much disabled as replaced. I would still expect that dragging a URL from the location bar would create a shortcut on the desktop.
Comment on attachment 335829 [details]
screenshot with the latest patch applied

As per previous comments.
Attachment #335829 - Flags: ui-review?(beltzner) → ui-review-
Comment on attachment 337351 [details] [diff] [review]
patch v12 - some small fixes

As per previous comments (re: interactions)
Attachment #337351 - Flags: ui-review?(beltzner) → ui-review-
(In reply to comment #75)
> (In reply to comment #66)
> > ...
> The shortcut for cloning in OSX is actually "option", so we should use that.
> On Windows ctrl is correct. The drag targeting should work as above.
OK, I will change that.

> > ...
> Foreground/background preferences aren't needed here and just muddy things up,
> IMO. I can't really see a good reason for dragging a tab and then leaving it in
> the background.
> 
> To duplicate within a tab strip, a opt/ctrl-drag with a target on the tabstrip
> should suffice. No need to target some small new tab button (which, I'll point
> out, is purely hypothetical at this point :)
> 
> > * Drag the tab over the new window button - to clone the tab to a new window.
> 
> New window button? No way.
Actually it wasn't my intention to implement the drag over new tab/window button thing at all (I think it isn't very useful, and there are better ways to get that behavior). But when I looked into the code I discovered that it already works exactly in the way I describe in comment #66 in current Firefox. The only difference with my patch is that the cloned tab doesn't retain history (because duplicateTab didn't exist when that was implemented). Should I remove the drag over new tab/window behavior completely than?

> (In reply to comment #70)
> > ...
> 
> That's fine, and Jim Mathies (cc'd on this bug already) has said that he'd be
> interested in helping out. Jim: what's needed here?
Can I file a followup in advance, or should I wait until this patch gets r+?

Your other comments are all things I think should be dealt with in the followup bug.
Flags: wanted-firefox3.1?
Flags: wanted-firefox3.1? → wanted-firefox3.1+
>> This is exactly how I intended to implement it, but the problem is that the
>> code that handles this is I think in C++. That makes it much harder to
>> implement it in that way. Maybe there could be filed a followup bug to make it
>> work like that.
>
>That's fine, and Jim Mathies (cc'd on this bug already) has said that he'd be
>interested in helping out. Jim: what's needed here?

I think it's a split off from the work going on here. To implement I think we'd just need to get the info down into the data object so it would know how to handle the data requests from apps and the desktop (drop a url, drop to open a new window). We could then reject data requests on the actual drop and spawn a new window from there.

The one thing that might cause problems are the drag feedback cursors, which change based on what our data object claims it's supports in terms of data flavors. We might be able to just fake out underlying apps by claiming we support everything. We'd have to do some experimentation to see how things look to see.

The animation stuff (was looking at Safari's tab drag today) might be a little tough :), but we could split that out as a separate bug and take a look to see what's possible.
>new window). We could then reject data requests on the actual drop and spawn a
>new window from there.

I'm not sure about preserving the dom - but if we can get enough data about the tab to the data object, and open up a line of communication to whatever would handle spawning that off? maybe? That's a little out of my realm.
Blocks: 455616
In this patch I did what is said in this comment:
> The shortcut for cloning in OSX is actually "option", so we should use that.
> On Windows ctrl is correct. The drag targeting should work as above.
First I was a little confused because the patch from bug 298571, also used metaKey(cmd) as a copy modifier, but it appears that that's also wrong, and that it will be fixed by bug 410569. For this patch I couldn't use the following line for the copy modifier (this line is what bug 410569 uses):
> var isCopy = aEvent.dataTransfer.dropEffect == "copy";
This has something to do with the new drag & drop api, which I think is not yet used for this code.

> Foreground/background preferences aren't needed here and just muddy things up,
> IMO. I can't really see a good reason for dragging a tab and then leaving it in
> the background.
> 
> To duplicate within a tab strip, a opt/ctrl-drag with a target on the tabstrip
> should suffice. No need to target some small new tab button (which, I'll point
> out, is purely hypothetical at this point :)
> 
> > * Drag the tab over the new window button - to clone the tab to a new window.
> 
> New window button? No way.
I haven't changed anything here yet, because I'm not sure what to do with it, see comment #78

So this is how the patch works now:
* Drag the tab to the content area of the browser - to detach the tab.
* Drag the tab to the content area of the browser while holding ctrl/option - to
clone the tab to a new window.
* Drag the tab over the new tab button - to clone the tab to the last position
on the tabbar, in background(depending on the pref).
* Drag the tab over the new tab button while holding shift - to clone the tab
to the last position
on the tabbar, selected(depending on the pref).
* Drag the tab over the new window button - to clone the tab to a new window.
* A clone tab menu option is added to the tab context menu, plus a corresponding commandkey: ctrl+shift+c. (see: attachment 335829 [details])

For implementing the other stuff you've mentioned I've opened bug 455616. Jim Mathies is already on the CC list of that bug, so discussion about that stuff can go further there.
Attachment #337351 - Attachment is obsolete: true
Attachment #338987 - Flags: ui-review?(beltzner)
Attachment #337351 - Flags: review?(dao)
Taking. This, as far as i can tell (and sort of test) does not need any platform change. Well, there's one thing we cannot do, which is to change the drag feedback only once you exit the tabbar. We could though change the drag feedback from what it's now to the tab preview as you start dragging the tab.
Assignee: klaas1988 → mano
No longer blocks: 455616
Depends on: 455616
Blocks: 455694
Attachment #338987 - Flags: ui-review?(beltzner)
Comment on attachment 338987 [details] [diff] [review]
patch v13 - use altKey instead of metaKey on mac

This is out of the scope for this bug, I'm going to use this bug for what is well outlined in comment 75. Code wise, this looks good, please spin this off to new bugs (esp. the modifiers supports)
No longer depends on: 455616
Blocks: 455722
(In reply to comment #79)
> The animation stuff (was looking at Safari's tab drag today) might be a little
> tough :), but we could split that out as a separate bug and take a look to see
> what's possible.
For this I've opened bug 455694.

(In reply to comment #83)
> (From update of attachment 338987 [details] [diff] [review])
> This is out of the scope for this bug, I'm going to use this bug for what is
> well outlined in comment 75. Code wise, this looks good, please spin this off
> to new bugs (esp. the modifiers supports)
The new tab menutem and the commandkey is now in bug 455722. The drag to new tab/window button part is also removed from this patch.

The only thing that needs to be added is the part that needs to be in C++, I thought it would be more clear to put that part in a separate bug that's why I opened bug 455616. What is exactly what you are going to implement, do you also plan to add the animated stuff?

What dis patch now does is:
* Drag the tab to the content area of the browser - to detach the tab.
* Drag the tab to the content area of the browser while holding ctrl/option -
to
clone the tab to a new window.
Attachment #335829 - Attachment is obsolete: true
Attachment #338987 - Attachment is obsolete: true
Attachment #339076 - Flags: ui-review?(beltzner)
Attachment #339076 - Flags: review?(mano)
Comment on attachment 339076 [details] [diff] [review]
patch v14 - remove the drag to new tab/window button stuff and the clone tab menuitem and commandkey

This is still an overkill for this bug. I'm working on a patch that does exactly what is described in comment 75, including the desktop support (minus the _dynamic_ change in the drag feedback). Please keep all of this stuff for followups.
Attachment #339076 - Flags: ui-review?(beltzner)
Attachment #339076 - Flags: ui-review-
Attachment #339076 - Flags: review?(mano)
Attachment #339076 - Flags: review-
Attachment #339076 - Flags: ui-review-
Priority: P2 → P1
Target Milestone: --- → Firefox 3.1b1
(In reply to comment #86)
> (From update of attachment 339076 [details] [diff] [review])
> This is still an overkill for this bug. I'm working on a patch that does
> exactly what is described in comment 75, including the desktop support (minus
> the _dynamic_ change in the drag feedback). Please keep all of this stuff for
> followups.

So basically everything except the detachTab method should go to a separate bug? Are you going to use something that looks like my detachTab method, or a completely different approach?
Priority: P1 → P2
Target Milestone: Firefox 3.1b1 → ---
Depends on: 455884
hey Mano - with bug 455884 landing, do you have anything you can post here so we can play around with this and see how it looks?
Attachment #339076 - Attachment is obsolete: true
Attached patch Initial Support (obsolete) — Splinter Review
Attachment #346262 - Flags: review?(mconnor)
(In reply to comment #90)
> Created an attachment (id=346262) [details]
> Initial Support

With this patch it isn't possible to clone a tab to a new window, by holding ctrl/option while dropping the tab on the content area or desktop. This is implemented in my patch and is also outlined in comment 75. Are you still planning to add this?
Also Mike Beltzner said a detach tab context menuitem isn't wanted (see: comment 62), so maybe that should be removed.
We need the context menu item for accessibility, at the very least. Clone tab might be implemented a little bit after this land.
(In reply to comment #92)
> Clone tab might be implemented a little bit after this land.

Should there be filed a followup bug for that?
Blocks: 463088
(In reply to comment #90)
> Created an attachment (id=346262) [details]
> Initial Support

Just posted a cursors patch in bug 463088 that compliments this for win32.
Comment on attachment 346262 [details] [diff] [review]
Initial Support

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
>+    else if (uriToLoad instanceof XULElement) {
>+      // swap the given tab with the default about:blank once and then close
>+      // the original tab in the other window.

s/once/tab/ for clarity?

>+      // We need to reset selectedTab after we've done
>+      // swapBrowsersAndCloseOther, so that the updateCurrentBrowser
>+      // it triggers will correctly update our URL bar.
>+      gBrowser.selectedTab = gBrowser.selectedTab;
>+      gBrowser.setTabTitle(gBrowser.selectedTab);

Mmm, why isn't this handled by swapBrowsersAndCloseOther ?

This looks ok to me, but I'd like Neil to take a second look.
Attachment #346262 - Flags: review?(mconnor)
Attachment #346262 - Flags: review?(enndeakin)
Attachment #346262 - Flags: review+
+                          oncommand="var tabbrowser = this.parentNode.parentNode.parentNode.parentNode;

Using getBindingParent would be simpler.

+            var height = Math.round(width * 0.6875); //16 : 11

Make the comment clearer as to what it means. It took me a moment to realize it was a ratio rather than commented out code.

+            var scale = width / win.innerWidth;
+            ctx.scale(scale, scale);
+            ctx.drawWindow(win, win.scrollX, win.scrollY,
+                           win.innerWidth, win.innerWidth * 0.6875, "rgb(255,255,255)");

The canvas itself doesn't seem to have a size set. It would end up it's default size and be too big or could be clipped on some displays, no?

Also, I think 'white' is clearer than the rgb colour.

+            dt.setDragImage(canvas, 25, 25);

Why 25,25?
Why haven't the strings for this landed yet, at least?
Sorry for spam: we've missed string freeze for beta 2, so we're going to have to leave the context menu aspect of this out for now. We can ask for late-l10n entry from the l10n team for the next milestone.
Comment on attachment 346262 [details] [diff] [review]
Initial Support

Seems there's a need to remove the context menu then.
Attachment #346262 - Flags: review?(enndeakin) → review-
Mano, can we get an updated patch?
Attached patch patch (obsolete) — Splinter Review
Attachment #346262 - Attachment is obsolete: true
Attachment #347740 - Flags: review?(enndeakin)
Attachment #347740 - Flags: review?(enndeakin) → review+
Attachment #347740 - Flags: approval1.9.1b2?
Flags: blocking-firefox3.1?
Comment on attachment 347740 [details] [diff] [review]
patch

a=beltzner, it'd be nice to have this for b2; co-ordinate landing with the sheriff
Attachment #347740 - Flags: approval1.9.1b2? → approval1.9.1b2+
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Attached patch for checkinSplinter Review
As discussed on IRC, adding back the context menu item, but with a temporary label (from places).
Attachment #347740 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/6b7faccfd305
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
With this patch, when dragging a tab (to a place where it would detach and form a new window), I get the following error:

Error: transferData.first is null
Source File: chrome://global/content/nsDragAndDrop.js
Line: 458
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081115 Minefield/3.1b2pre
I notice on Windows when dragging and dropping a tab off the Firefox window the mouse turns in to a stop symbol, this isn't very intuitive. Filed bug 465153
This appears to not work in Vista.  I can drag a tab to taskbar or to the desktop and a Firefox window will open, but the tab that was dragged into the window does not 'load', only leaving a blank window.

There are several errors in the Console2, but right now I have not time to ID the ones related to the drag&drop.
File a bug and please do try with a new profile.
Blocks: 465167
Keywords: uiwanted
Whiteboard: [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] [has patch] [needs review beltzner] → [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
Blocks: 465179
Blocks: 465182
Depends on: 465184
No longer blocks: 465167, 465179, 465182
Depends on: 465182, 465179, 465167
Target Milestone: --- → Firefox 3.1b2
Depends on: 465185
Depends on: 465186
I'd appreciate if discussions about strings were documented in a better way than merely "as discussed on irc".
Wouldn't it be better to copy the tab to the new window? Or else I think it would be more clear to use the text Cut tab to new window. I was really surprised to see the tab disappear, unable to get quickly back the previous order.
No longer depends on: 465185
Depends on: 465231
Mano, can this have caused bug 465256?
Depends on: 465332
Depends on: 465333
Depends on: 465256
I can no longer drag a bookmark to a different place on the bookmark menu, instead I get the error:

Assertion Failed
ASSERT: copy or move action without a tab
Stack Trace: 
0:_onDrop([object DragEvent])
1:ondrop([object DragEvent])

Filed it as Bug 465333, I assume this is a regression from this bug.
What is the purpose of the browser.xul portion of this patch?  It does not seem to relate to this feature at all and causes the regression reported in bug 465231.
No longer depends on: 465153
This should be backed out if it's causing this many regressions in beta 2. Period.
(In reply to comment #114)
> This should be backed out if it's causing this many regressions in beta 2.
> Period.

Talked with Mano on IRC, he says he's got a patch that addresses the various regressions. Re-opening and putting this back on the blocker list. If we can't get that patch in for beta 2, we'll back it out and do it for the next milestone.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Fix most major issues (obsolete) — Splinter Review
* Toolbar customization
 * Places D&D (toolbar & menu)
 * Dragging a tab on itself doesn't create a new window
 * "Don't show the tabbar when only one tab is visible" issue is mostly fixed.
Attachment #348673 - Flags: review?(mconnor)
Flags: in-litmus?
Whiteboard: [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] → [needs review mconnor][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
some drive-by comment:
> +++ b/browser/components/places/content/menu.xml

while there, why not stopping propagation also for dragstart and dragover? Aren't we catching them by error in toolbar.xml aswell?

> diff --git a/browser/components/places/content/toolbar.xml b/browser/components/places/content/toolbar.xml

+        var ip = this.insertionPoint;
+        if (!ip || !PlacesControllerDragHelper.canDrop(ip))
           return;
-
+        
 trailing spaces here

       <handler event="dragleave"><![CDATA[
+        // Only handle dragleaves for the toolbar itself (and not for its child
+        // nodes)
+        if (event.target != this)
+          return;
+
         PlacesControllerDragHelper.currentDropTarget = null;
-        PlacesControllerDragHelper.currentDataTransfer = null;

why not unsetting here? it will be set again as soon as we over a valid Places target, isn't it?
Comment on attachment 348673 [details] [diff] [review]
Fix most major issues

>+            var tabListenerBlank = null

lacks a semicolon
Flags: in-litmus? → in-litmus+
Comment on attachment 348673 [details] [diff] [review]
Fix most major issues


>+            var ourBrowser = this.getBrowserForTab(aOurTab);
>+
>             // Unhook our progress listener
>             var ourIndex = aOurTab._tPos;
>+            // Usually, there's no listener or filter if we're not in tabbed-mode

bug 463384 should make this bit of this patch unnecessary, no?  It shouldn't be harmful, either way.


>+            // don't interfere toolbar customization

nit: don't interfere with toolbar customization

Looks good. r=mconnor
Attachment #348673 - Flags: review?(mconnor) → review+
Attachment #348673 - Flags: approval1.9.1b2?
Whiteboard: [needs review mconnor][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] → [needs landing][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
(In reply to comment #117)
> some drive-by comment:
> > +++ b/browser/components/places/content/menu.xml
> 
> while there, why not stopping propagation also for dragstart and dragover?
> Aren't we catching them by error in toolbar.xml aswell?
> 

fixed.

> > diff --git a/browser/components/places/content/toolbar.xml b/browser/components/places/content/toolbar.xml
> 
> +        var ip = this.insertionPoint;
> +        if (!ip || !PlacesControllerDragHelper.canDrop(ip))
>            return;
> -
> +        
>  trailing spaces here
> 
that too.

>        <handler event="dragleave"><![CDATA[
> +        // Only handle dragleaves for the toolbar itself (and not for its
> child
> +        // nodes)
> +        if (event.target != this)
> +          return;
> +
>          PlacesControllerDragHelper.currentDropTarget = null;
> -        PlacesControllerDragHelper.currentDataTransfer = null;
> 
> why not unsetting here? it will be set again as soon as we over a valid Places
> target, isn't it?

Added that back, but i'll note that this field is not very reliable from my
testing (for some reason you get dragleave before the actual drop, so you end
up with a null field, then we set it in drop)

(In reply to comment #120)
> (From update of attachment 348673 [details] [diff] [review])
> 
> >+            var ourBrowser = this.getBrowserForTab(aOurTab);
> >+
> >             // Unhook our progress listener
> >             var ourIndex = aOurTab._tPos;
> >+            // Usually, there's no listener or filter if we're not in tabbed-mode
> 
> bug 463384 should make this bit of this patch unnecessary, no?  It shouldn't be
> harmful, either way.
> 

Removed as it was only half-working anyway. I have double-checked and the bug i
tried to fix here is ixed by the patch from bug 4643384.

I've also tweaked _setEfffects... not to set the effectAllowed field when a
non-tab-drag is happening on the toolbox.
Attachment #348673 - Attachment is obsolete: true
Attachment #348673 - Flags: approval1.9.1b2?
http://hg.mozilla.org/mozilla-central/rev/4f32d50c10bc
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] → [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
Depends on: 465597
Depends on: 465608
Depends on: 465588
Not sure where to report this, as it is no doubt a fall-out from this bug.  If someone could point me in the right direction, that would be appreciated.

I'm getting random cases of stuck tabs that can't be closed.  Attempts to close it generate this exception:

Error: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebProgress.removeProgressListener]
Source file: chrome://browser/content/tabbrowser.xml
Line: 1509

I've experienced this both with the nightly and now the current hourly:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081118 Minefield/3.1b2pre ID:20081118181119
IU: see bug 465167, if it's not the same thing, file a new bug and mark it as blocking this one.
Depends on: 465685
Depends on: 465693
When exactly does this patch land? Using the Tinderbox build:

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre ID:20081119041246

Not a single regression from this bug has been fixed.
Scratch the above comment, some bugs have been fixed I was just testing a few and forgot the mouse thing hadn't been fixed, I'll just re-open some.
Re Comment 116: using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081119 Minefield/3.1b2pre which was 
built from changeset http://hg.mozilla.org/mozilla-central/rev/0cd41f599080, the Toolbar Customize dialog is blank when I tear away a tab on Mac. Did the fix not make the build I am testing?
After some debugging help, aki pointed me to http://pastebin.mozilla.org/573061 which indicates that the nightly build did not have the merge.

(In reply to comment #128)
[snip]
Depends on: 465883
Depends on: 465910
I'm not quite clear on the intended behaviour here...

>> also because the create shortcut by
>> dragging to desktop behavior needs to be disabled then
> Not as much disabled as replaced. I would still expect that dragging a
> URL from the location bar would create a shortcut on the desktop.

Does this mean that it is intended that dragging a tab to anywhere except elsewhere on the tab bar means "create a new window"?  In particular, is it intended that dragging a tab to the home button replaces the previous "set as home page" behaviour, and dragging a tab to the bookmarks bar or menu replaces the previous behaviour of "add a bookmark"?

If so, bug 465910 and bug 465332 should be WONTFIX - I expect they may attract a number of reports in beta 2
No longer depends on: 465883
Version: unspecified → Trunk
Depends on: 466140
Depends on: 465704
Depends on: 466350
No longer depends on: 466350
Depends on: 466371
No longer depends on: 466371
Depends on: 466616
No longer depends on: 466616
Blocks: 461526
Blocks: 461637
> >> also because the create shortcut by
> >> dragging to desktop behavior needs to be disabled then
> > Not as much disabled as replaced. I would still expect that dragging a
> > URL from the location bar would create a shortcut on the desktop.

Michael - A tab should open a new window when dropped, except in the following circumstances:

Behavior: Tab is dropped over the home button
Result: Tab location becomes "home" location (Bug 465910)

Behavior: Tab is dropped over bookmark toolbar
Result: Cursor changes to insertion marker on hover, tab is added to bookmark toolbar in place when dropped (Bug 465332)

Behavior: Tab is dropped over "new tab" button
Result: Tab is created to the last position on the tabbar and given focus (Comment 75)

Behavior: Drop tab in different location in tab strip
Result: Tab moves to that location

Behavior: Drop tab into tab strip of another window
Result: Tab is inserted into that filmstrip

I think that's all of them.
(In reply to comment #131)

On the bookmark menu as well should be an exception...
(In reply to comment #132)
> (In reply to comment #131)
> 
> On the bookmark menu as well should be an exception...

I originally intended Bug 465332 to cover all bookmark areas, but Beltzner morphed it :(.
It seems to me that the simplest scenario for tab tear-off would have been for a user to simply drag a tab below the tabbar (i.e. into the content area) and release.  Short, simple, and all other tab dragging operations are not messed up.  The action would also be intuitive: like a tearing off.
(In reply to comment #131)
> I think that's all of them.

Not really.  If you consider icons that are not on the toolbar by default.  Before this landed, it was possible to clone a tab in a new window by dragging it to the New Window icon.

It would seem to make sense to me to be able to print the contents of a tab by dragging the tab to the printer icon.

The real problem of making it a dragging it off the tabbar to anyplace not on the tabbar detaches the tab.  It makes it impossible to ever implement any of these other things, most of which, make logical sense.

It can not be a anyplace you drag it to except becuase then each time you add a new drag a tab feature you add a new exception and that means breaking the way people used to detach tabs.

If some toolbar icons are to be exceptions, then it makes more sense to not have the detach occur if the tab is dragged to a toolbar at all.  Not a some icons on the toolbar do and some do not do the detach.
(In reply to comment #134)
> It seems to me that the simplest scenario for tab tear-off would have been for
> a user to simply drag a tab below the tabbar (i.e. into the content area) and
> release.  Short, simple, and all other tab dragging operations are not messed
> up.  The action would also be intuitive: like a tearing off.

Yes, this is the intended behavior.

(In reply to comment #135)
> it was possible to clone a tab in a new window by dragging
it to the New Window icon.
>It would seem to make sense to me to be able to print the contents of a tab by
dragging the tab to the printer icon.

I agree, but these buttons aren't default chrome so I'd consider implementing them not as high priority as getting the main chrome items to work.

>It can not be a anyplace you drag it to except becuase then each time you add a
new drag a tab feature you add a new exception and that means breaking the way
people used to detach tabs.

Each of the items we're talking about as a target is a few pixels wide and in the chrome.  The target for the open in new tab behavior is still *everywhere else.*  You could hit it with a club.

(In reply to comment #132)
>On the bookmark menu as well should be an exception...

Good point, I think that would be awesome and could even become a great main way to create and sort a bookmark.  Compare the time required to create and sort a bookmark now (click star, click again, find folder, expand list if you don't see it, click done) to the click and drag you're proposing.  Pretty nice.  That may not be a part of this bug though or this implementation though (Beltzner's the one to answer that).
(In reply to comment #136)
> (In reply to comment #132)
> >On the bookmark menu as well should be an exception...
> 
> Good point, I think that would be awesome and could even become a great main
> way to create and sort a bookmark. [...]
> That may not be a part of this bug though

Said behavior got broken in this bug.
You can still bookmark sites by dragging the favicon to the bookmarks bar.  It's a bit broken at the moment (bug 312852) but I find it more convenient than using the star.
exactly all of those behaviours were possible before, you can drag the favicon, but that works only if your actually looking at the tab you want to bookmark. What changed is only tab dragging, previously dragging a tab was creating an "url" flavor, so every target accepting urls was working, actually it does not, and that's by design to avoid making tab detach a task similar to aiming to a beer can with a catapult. So i think the issue is find a target large enough to make detach an easy task, without making everything a target.
Depends on: 467376
Depends on: 467491
IMHO the "url" flavor should be added back to fix a good portion of the bugs blocking this one, only the content window should not accept "url" flavor in the case of an xul:tab being dropped. IOW, you'll have to accept the "url" flavor in the content window for regular url drops, just make a check for the tab-dragging case where you would discard the drop. Also any time the drag drop isn't accepted you can just spawn a new window, this should pretty much cover all the cases, if I understand this correctly. I'll try to whip something up if no one beats me to it. ;)
Blocks: 468075
No longer blocks: 468075
Maybe it is best to add "application/x-moz-tabbrowser-tab" flavor to homeButtonObserver, bookmarksButtonObserver and all other observers?

Did anyone noticed that tab icon is not updated for the detached tab?
(In reply to comment #141)
> Did anyone noticed that tab icon is not updated for the detached tab?

should be bug 449730
Depends on: 468553
Depends on: 468537
No longer depends on: 468537
Depends on: 469176
Blocks: 469640
No longer blocks: 469640
Depends on: 469640
Depends on: 469643
No longer depends on: 469643
Blocks: 460666
Depends on: 470189
No longer depends on: 467491
By seeing the list of not satisfied users, I believe that this bug can be marked as verified on trunk and 1.9.1. The patch has been landed a long time ago and hasn't caused a perf regression. All remaining work is covered by depending bugs.
Status: RESOLVED → VERIFIED
(In reply to comment #104)
> http://hg.mozilla.org/mozilla-central/rev/6b7faccfd305

This patch landed with temporary workaround for a tab context menu item, where the item is using the 'Open in a new window' string taken from places, moreover with no accesskey. Is there a bug already filed for this? Now there's b4 on the way we should really fix this for 3.1 release.
Depends on: 482450
Depends on: 490031
Depends on: 495450
Depends on: 477797
No longer depends on: 499523
Depends on: 559574
No longer blocks: 455722
Blocks: 449728
You need to log in before you can comment on or make changes to this bug.