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)
Firefox
Tabbed Browser
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:
Comment 1•21 years ago
|
||
this is firebird version of Bug 102132
Updated•21 years ago
|
Summary: Option to detach the tab → Ability to detach tabs
Comment 2•21 years ago
|
||
I quick search on "tab" didn't turn up any duplicates.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•21 years ago
|
||
*** 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.
Comment 5•21 years ago
|
||
*** Bug 234304 has been marked as a duplicate of this bug. ***
Comment 6•20 years ago
|
||
*** Bug 268329 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•20 years ago
|
QA Contact: firefox.general
Summary: Ability to detach tabs → Ability to detach tabs (Open a tab in a new window)
Assignee | ||
Updated•20 years ago
|
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.)
Updated•20 years ago
|
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)
Comment 8•20 years ago
|
||
*** Bug 293675 has been marked as a duplicate of this bug. ***
Comment 9•19 years ago
|
||
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.
Comment 10•19 years ago
|
||
*** Bug 302413 has been marked as a duplicate of this bug. ***
Comment 11•19 years ago
|
||
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.
Comment 12•19 years ago
|
||
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.
Comment 13•19 years ago
|
||
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".
Reporter | ||
Comment 14•19 years ago
|
||
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.
Comment 15•19 years ago
|
||
This requires Gecko changes that won't happen on the 1.8 branch for Fx2
Flags: blocking-aviary2? → blocking-aviary2-
Updated•19 years ago
|
Assignee: bugs → nobody
Comment 16•19 years ago
|
||
What Gecko changes does this depend on? Does it depend on bug 254144?
Comment 17•18 years ago
|
||
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!
Comment 18•18 years ago
|
||
*** Bug 359839 has been marked as a duplicate of this bug. ***
Comment 20•18 years ago
|
||
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.
Comment 21•18 years ago
|
||
(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 ;)
Comment 22•18 years ago
|
||
> 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 ;)
Reporter | ||
Comment 23•18 years ago
|
||
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...
Comment 24•18 years ago
|
||
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.
Reporter | ||
Comment 25•18 years ago
|
||
(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.)
Comment 26•18 years ago
|
||
(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.
Comment 28•18 years ago
|
||
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.
Comment 29•18 years ago
|
||
> 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).
Comment 30•18 years ago
|
||
Would adding "Open Tab in New Window" to the tab context menu also destroy the DOM changes?
Comment 31•18 years ago
|
||
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!
Updated•17 years ago
|
Priority: -- → P2
Whiteboard: PRD:TAB-003a parity-safari
Comment 34•17 years ago
|
||
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?
Comment 35•17 years ago
|
||
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.
Comment 36•17 years ago
|
||
(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?
Comment 37•17 years ago
|
||
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 ;-)
Updated•16 years ago
|
Comment 39•16 years ago
|
||
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)
Comment 40•16 years ago
|
||
Attachment #332736 -
Flags: ui-review?(beltzner)
Comment 41•16 years ago
|
||
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 42•16 years ago
|
||
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)
Comment 43•16 years ago
|
||
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)
Comment 44•16 years ago
|
||
Attachment #332878 -
Attachment is obsolete: true
Attachment #332952 -
Flags: ui-review?(beltzner)
Attachment #332878 -
Flags: ui-review?(beltzner)
Comment 45•16 years ago
|
||
Attachment #332736 -
Attachment is obsolete: true
Attachment #332953 -
Flags: ui-review?(beltzner)
Attachment #332736 -
Flags: ui-review?(beltzner)
Comment 46•16 years ago
|
||
(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.
Comment 47•16 years ago
|
||
(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)
Comment 48•16 years ago
|
||
Attachment #332953 -
Attachment is obsolete: true
Attachment #332966 -
Flags: ui-review?(beltzner)
Attachment #332953 -
Flags: ui-review?(beltzner)
Comment 49•16 years ago
|
||
(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.
Comment 50•16 years ago
|
||
(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.
Comment 51•16 years ago
|
||
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)
Comment 52•16 years ago
|
||
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)
Comment 53•16 years ago
|
||
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)
Comment 54•16 years ago
|
||
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#
Comment 55•16 years ago
|
||
It would be good to refactor the swap-and-close code instead of duplicating it from the drag/drop handler...
Comment 56•16 years ago
|
||
(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);
Comment 57•16 years ago
|
||
(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)
Comment 58•16 years ago
|
||
Either that or having another method that does that and swaps. I'd check with gavin on which he prefers, probably.
Comment 59•16 years ago
|
||
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);
Comment 60•16 years ago
|
||
I have no idea. You'd have to ask someone who's actually familiar with this code...
Comment 61•16 years ago
|
||
(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 62•16 years ago
|
||
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-
Comment 63•16 years ago
|
||
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)
Comment 64•16 years ago
|
||
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)
Comment 65•16 years ago
|
||
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)
Comment 66•16 years ago
|
||
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)
Comment 67•16 years ago
|
||
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)
Updated•16 years ago
|
Attachment #335827 -
Attachment description: patch v10 - add a clone tab menu item → patch v11 - add a clone tab menu item
Updated•16 years ago
|
Attachment #335827 -
Flags: review?(dao)
Comment 68•16 years ago
|
||
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?
Comment 69•16 years ago
|
||
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.
Comment 70•16 years ago
|
||
(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).
Comment 71•16 years ago
|
||
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.
Comment 72•16 years ago
|
||
(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.
Comment 74•16 years ago
|
||
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)
Updated•16 years ago
|
Keywords: uiwanted
Whiteboard: PRD:TAB-003a parity-safari → [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] [has patch] [needs review beltzner]
Comment 75•16 years ago
|
||
(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 76•16 years ago
|
||
Comment on attachment 335829 [details]
screenshot with the latest patch applied
As per previous comments.
Attachment #335829 -
Flags: ui-review?(beltzner) → ui-review-
Comment 77•16 years ago
|
||
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-
Comment 78•16 years ago
|
||
(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?
Updated•16 years ago
|
Flags: wanted-firefox3.1? → wanted-firefox3.1+
Comment 79•16 years ago
|
||
>> 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.
Comment 80•16 years ago
|
||
>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.
Comment 81•16 years ago
|
||
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)
Assignee | ||
Comment 82•16 years ago
|
||
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 | ||
Updated•16 years ago
|
Assignee: klaas1988 → mano
Updated•16 years ago
|
Assignee | ||
Updated•16 years ago
|
Attachment #338987 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 83•16 years ago
|
||
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)
Comment 85•16 years ago
|
||
(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)
Assignee | ||
Comment 86•16 years ago
|
||
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-
Assignee | ||
Updated•16 years ago
|
Attachment #339076 -
Flags: ui-review-
Assignee | ||
Updated•16 years ago
|
Priority: P2 → P1
Target Milestone: --- → Firefox 3.1b1
Comment 88•16 years ago
|
||
(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 → ---
Comment 89•16 years ago
|
||
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?
Updated•16 years ago
|
Attachment #339076 -
Attachment is obsolete: true
Assignee | ||
Comment 90•16 years ago
|
||
Attachment #346262 -
Flags: review?(mconnor)
Comment 91•16 years ago
|
||
(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.
Assignee | ||
Comment 92•16 years ago
|
||
We need the context menu item for accessibility, at the very least. Clone tab might be implemented a little bit after this land.
Comment 93•16 years ago
|
||
(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?
Comment 94•16 years ago
|
||
(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 95•16 years ago
|
||
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+
Comment 96•16 years ago
|
||
+ 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?
Comment 97•16 years ago
|
||
Why haven't the strings for this landed yet, at least?
Comment 98•16 years ago
|
||
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 99•16 years ago
|
||
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-
Comment 100•16 years ago
|
||
Mano, can we get an updated patch?
Assignee | ||
Comment 101•16 years ago
|
||
Attachment #346262 -
Attachment is obsolete: true
Attachment #347740 -
Flags: review?(enndeakin)
Updated•16 years ago
|
Attachment #347740 -
Flags: review?(enndeakin) → review+
Assignee | ||
Updated•16 years ago
|
Attachment #347740 -
Flags: approval1.9.1b2?
Assignee | ||
Updated•16 years ago
|
Flags: blocking-firefox3.1?
Comment 102•16 years ago
|
||
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+
Updated•16 years ago
|
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Assignee | ||
Comment 103•16 years ago
|
||
As discussed on IRC, adding back the context menu item, but with a temporary label (from places).
Attachment #347740 -
Attachment is obsolete: true
Assignee | ||
Comment 104•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 105•16 years ago
|
||
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
Comment 106•16 years ago
|
||
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
Comment 107•16 years ago
|
||
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.
Assignee | ||
Comment 108•16 years ago
|
||
File a bug and please do try with a new profile.
Updated•16 years ago
|
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]
Updated•16 years ago
|
Comment 109•16 years ago
|
||
I'd appreciate if discussions about strings were documented in a better way than merely "as discussed on irc".
Comment 110•16 years ago
|
||
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.
Comment 111•16 years ago
|
||
Mano, can this have caused bug 465256?
Comment 112•16 years ago
|
||
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.
Comment 113•16 years ago
|
||
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.
Comment 114•16 years ago
|
||
This should be backed out if it's causing this many regressions in beta 2. Period.
Comment 115•16 years ago
|
||
(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 → ---
Assignee | ||
Comment 116•16 years ago
|
||
* 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)
Updated•16 years ago
|
Flags: in-litmus?
Updated•16 years ago
|
Whiteboard: [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] → [needs review mconnor][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
Comment 117•16 years ago
|
||
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 118•16 years ago
|
||
Comment on attachment 348673 [details] [diff] [review]
Fix most major issues
>+ var tabListenerBlank = null
lacks a semicolon
Comment 119•16 years ago
|
||
https://litmus.mozilla.org/show_test.cgi?id=7436 added to Litmus.
Updated•16 years ago
|
Flags: in-litmus? → in-litmus+
Comment 120•16 years ago
|
||
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+
Assignee | ||
Updated•16 years ago
|
Attachment #348673 -
Flags: approval1.9.1b2?
Updated•16 years ago
|
Whiteboard: [needs review mconnor][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] → [needs landing][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
Assignee | ||
Comment 121•16 years ago
|
||
(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.
Assignee | ||
Comment 122•16 years ago
|
||
Attachment #348673 -
Attachment is obsolete: true
Attachment #348673 -
Flags: approval1.9.1b2?
Assignee | ||
Comment 123•16 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs landing][PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera] → [PRD:TAB-003a] [parity-safari] [parity-chrome] [parity-opera]
Comment 124•16 years ago
|
||
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
Comment 125•16 years ago
|
||
IU: see bug 465167, if it's not the same thing, file a new bug and mark it as blocking this one.
Comment 126•16 years ago
|
||
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.
Comment 127•16 years ago
|
||
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.
Comment 128•16 years ago
|
||
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?
Comment 129•16 years ago
|
||
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]
Comment 130•16 years ago
|
||
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
Updated•16 years ago
|
Version: unspecified → Trunk
Comment 131•16 years ago
|
||
> >> 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.
Comment 132•16 years ago
|
||
(In reply to comment #131)
On the bookmark menu as well should be an exception...
Comment 133•16 years ago
|
||
(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 :(.
Comment 134•16 years ago
|
||
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.
Comment 135•16 years ago
|
||
(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.
Comment 136•16 years ago
|
||
(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).
Comment 137•16 years ago
|
||
(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.
Comment 138•16 years ago
|
||
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.
Comment 139•16 years ago
|
||
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.
Comment 140•16 years ago
|
||
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. ;)
Comment 141•16 years ago
|
||
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?
Comment 142•16 years ago
|
||
(In reply to comment #141)
> Did anyone noticed that tab icon is not updated for the detached tab?
should be bug 449730
Updated•16 years ago
|
Keywords: fixed1.9.1
Updated•16 years ago
|
Comment 143•16 years ago
|
||
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
Keywords: fixed1.9.1 → verified1.9.1
Comment 144•16 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•