Closed Bug 455694 Opened 16 years ago Closed 13 years ago

Implement animation for tab reordering and detaching (via drag and drop)

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 674925
Firefox 8

People

(Reporter: klaas1988, Unassigned)

References

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

Details

(Keywords: ux-userfeedback, Whiteboard: [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie])

Attachments

(3 files, 40 obsolete files)

6.02 KB, text/html
Details
5.84 KB, patch
Details | Diff | Splinter Review
60.13 KB, patch
Details | Diff | Splinter Review
When you drag and drop a tab on the tabbar in Safari and Chrome you see a nice animation, the tab is moved to the place where the mouse cursor is before you drop it. Currently Firefox only shows a small arrow to indicate where the tab will be dropped. When you drag the the tab outside the toolbar in those browsers (which "detaches" the tab) the tab changes into a thumbnail of that tabs content.

This bug is for implementing the same behavior in Firefox.

From bug 225680 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.
So here it is!
Whiteboard: [parity-safari] [parity-chrome]
Maybe the possibility to use "arbitrary elements as paint servers for css backgrounds" that roc plans to add, could be used to create the detach tab thumbnail. This could be used instead of something like canvas draWindow. See: http://weblogs.mozillazine.org/roc/archives/2008/07/the_latest_feat.html
As far as I know, Mano has the thumbnail preview working in bug 225680 now. What needs to be done is animating the other tabs on the tabstrip so that they slide gracefully as the dragged tab is moved left/right.

(I'll be honest; I'm finding it hard to decode what parts of this are bug 225680 and what parts are here.)
Depends on: 455884
No longer depends on: 455884
Hardware: PC → All
This looks like a duplicate of this: https://bugzilla.mozilla.org/show_bug.cgi?id=410972
Blocks: 552047
(In reply to comment #8)
> Any chances to get this finalized in Firefox 4 ? ;)

I don't see why not. It should block the final release at least
(In reply to comment #9)
> (In reply to comment #8)
> > Any chances to get this finalized in Firefox 4 ? ;)
> I don't see why not. It should block the final release at least

The animated mockups are here : http://blog.stephenhorlander.com/2010/01/29/tab-animation/
Shouldn't this be split up into two bugs? Tab dragging animation would be easier to handle first, then tab dropping.
Blocks: 582678
No longer blocks: 582678
(In reply to comment #12)
> Demo:
> http://people.mozilla.com/~fyan/prototypes/tabslide.html

That looks very nice. I really look forward to seeing this in a browser.
This demo is the most beautiful thing that I've ever seen since the first time that I saw Tab Candy!  I'd really like to see this in Firefox 4.  Please keep up the good work!  I'll try to lurk around in the bug and make myself useful by giving drive-by reviews and such.
Frank, you're the man.

How are you going to handle the case where you drag a tab out of an almost-overflowing tabstrip? Ideally the other tabs would grow back with animation, and you can't do that just with transforms.
Thanks guys. :)
When I have time, there are other bits that I'd like to add to the prototype, including overflow handling and a smoother detach animation. Code contributions are welcome. ;)

(In reply to comment #15)
> How are you going to handle the case where you drag a tab out of an
> almost-overflowing tabstrip? Ideally the other tabs would grow back with
> animation, and you can't do that just with transforms.

This is debatable, but my intuition is that the tabs should not grow back until the detached tab is dropped. The UX advantage in bug 465086 of deferring the resize does not apply here, but I do think that it is less jarring anyway.
(In reply to comment #16)
> Created attachment 463283 [details]
> my prototype (mirror of link in comment 12)
Holy crap that is awesome!  I'm even going to request blocking because of the awesomeness of it bringing some huge polish to rearranging tabs.
blocking2.0: --- → ?
What does it take to go from prototype to patch, here? (So sad that it's not "same technology, same difference!")

I wouldn't hold the release on this, but have wanted this since Firefox 2, so you can bet your bippy that I'd approve the heck out of a patch that came along with tests and no performance impact.
how will this work when ctrl is pressed for duplicating the tab?
Attached file tab dragging in XUL (obsolete) —
I made the mistake of making a crack about how it was sad that it's such a pain to do this kind of stuff in XUL in front of Neil Deakin, and he made this XUL version/translation.

Frank: does this help you get to a patch?

(Neil: you are awesome)
(In reply to comment #20)
> Created attachment 463754 [details]
> tab dragging in XUL
>
> Frank: does this help you get to a patch?

Thanks for the translation, Neil.

Beltzner, my HTML prototype used display: -moz-box, which is the XUL box model, so I had an idea of where to go next, but having it already coded for me is always fantastic :)

Before I put together a patch, I have some more ideas up my sleeve. ;)
(In reply to comment #21)
> (In reply to comment #20)
> > Created attachment 463754 [details] [details]
> > tab dragging in XUL
> >
> > Frank: does this help you get to a patch?
> 
> Thanks for the translation, Neil.
> 
> Beltzner, my HTML prototype used display: -moz-box, which is the XUL box model,
> so I had an idea of where to go next, but having it already coded for me is
> always fantastic :)
> 
> Before I put together a patch, I have some more ideas up my sleeve. ;)

Perfect is the enemy of good! I want this piece already!
Don't think I can block on this, but I really really want it; I will continue to help it forward as I can, up to and including approvals!
blocking2.0: ? → -
Keywords: ux-feedback
Depends on: 455722
I'm a bit new to this game, so I don't really know when feature freeze is, but is this going to make it? Anyone working on this?
(In reply to comment #25)
> I'm a bit new to this game, so I don't really know when feature freeze is, but
> is this going to make it? Anyone working on this?

There's no assignee, so nobody is working on this.
(In reply to comment #23)
> Don't think I can block on this, but I really really want it; I will continue
> to help it forward as I can, up to and including approvals!

Isn't it a key part of the Fx4 facelift as many changes happenned to tabs with Fx4 final as a target : UX changes, position (on top) and the animation when openning/closing tab ?

At least could we just implement animation for reordering tab for Fx4 as I guess it's a more common action and that I saw in a previous nightly (on Linux and mac) that the "new tab's position" indicator was truncated because of tab-on-top ? (yes mean reop bug 410972 and target it to Fx4)
No longer blocks: 552047
I think it's noteworthy to look at how IE9 handles the animation for tear off tabs. It's quite interesting
(In reply to comment #28)
> I think it's noteworthy to look at how IE9 handles the animation for tear off
> tabs. It's quite interesting

Could you catch that and send it (blog, attachement, ...) or at least explain what IE9 do (I only have OSX and Linux desktops) please ?
[parity-IE] should be added, right ? ;-)
In IE9 beta, tab reordering shows a sliding animation just like the prototype attached to this bug, and like Chrome and Safari.
Whiteboard: [parity-safari] [parity-chrome] → [parity-safari][parity-chrome][parity-ie]
As this bug cover reodering and tear-off animations of tabs, I propose to change "animation" to animationS" in the title.
To actually make this searchable, how about "Implement animations for tab reordering and tab tearing".
(In reply to comment #29)
> Could you catch that and send it (blog, attachement, ...) or at least explain
> what IE9 do (I only have OSX and Linux desktops) please ?

The rest of the UI elements disappear so that only the tab and page content are visible. There's no animation like in Chrome where the thumbnail's small while dragging.

http://img189.imageshack.us/img189/989/clipboard01dd.png
Very seamless 

(I'd make a video if I could find a program that makes webm video)
(In reply to comment #33)
> To actually make this searchable, how about "Implement animations for tab
> reordering and tab tearing".

So, should block bug 466196 ;-)
Depends on: 596954
Blocks: 596954
No longer depends on: 596954
Summary: Implement animation for drag & dropping of tabs → Implement animation for tab reordering (by dragging and dropping)
Attached video IE9 beta tab animations (obsolete) —
Video of IE9 beta tab animations per comment 34
I think this video is more related to Bug 485105 and Bug 485105 .
Sorry, and to Bug 504847 .
is something going to happen with this bug?
(In reply to comment #39)
> is something going to happen with this bug?

After Firefox 4, I might pick it up.
How close are we to finishing this, Frank? Any chance we could pick it up before Firefox 4, or is it pretty far away?
(In reply to comment #41)
> How close are we to finishing this, Frank? Any chance we could pick it up
> before Firefox 4, or is it pretty far away?

A dogfood-able patch should be quickly do-able. My concern is about polish and review, but if you and the team think this is something that we'd really want for Firefox 4 (and I do), then I'm definitely willing to invest time in it.
Definitely a big polish win for an area where we currently look clunky. If you can work out a dogfoodable patch we can toss to tryserver and play with in that build I think it'd be worthwhile, yes.
Blocks: 606803
Assignee: nobody → fryn
Status: NEW → ASSIGNED
Attached patch WIP v1 (obsolete) — Splinter Review
Mostly dogfoodable work-in-progress. There will be many changes to come.
I am well aware of the issues, so no need to report anything yet.
I'll throw up tryserver builds once this is closer to feature complete.
Blocks: 485105
No longer blocks: 606803
No longer depends on: 455722
Summary: Implement animation for tab reordering (by dragging and dropping) → Implement animation for tab reordering and detaching (by dragging and dropping)
Blocks: cuts-tabs
Summary: Implement animation for tab reordering and detaching (by dragging and dropping) → Implement animation for tab reordering and detaching (via drag and drop)
Does this bug count as a new feature? As in, could it still land for firefox 4.0 since feature freeze has happened?
See comments #42 and #43 right above you?
Yeah but those comments were before beta 7...
(In reply to comment #48)
> Yeah but those comments were before beta 7...

Tabs is toolbar is currently in tryserver. There's also an issue with drop markets on Mac, thus the plan is to get this, along with the tabs in toolbar for systems that support it, in time for the next beta.
What is "tabs in toolbar" ? :S
(In reply to comment #49)
> Tabs is toolbar is currently in tryserver. There's also an issue with drop
> markets on Mac, thus the plan is to get this, along with the tabs in toolbar
> for systems that support it, in time for the next beta.

There is zero chance of this making beta 8. Reordering animations will only partially resolve dropmarker issues. Cropped dropmarkers will still appear when dragging links to the tab bar, until we address that too.
(In reply to comment #50)
> What is "tabs in toolbar" ? :S

Sorry meant tabs in titlebar (bug 572160).

(In reply to comment #51)
> (In reply to comment #49)
> > Tabs is toolbar is currently in tryserver. There's also an issue with drop
> > markets on Mac, thus the plan is to get this, along with the tabs in toolbar
> > for systems that support it, in time for the next beta.
> 
> There is zero chance of this making beta 8. Reordering animations will only
> partially resolve dropmarker issues. Cropped dropmarkers will still appear when
> dragging links to the tab bar, until we address that too.

My apologies for the misinformation and thank you for the correction.
Nomming for blocking again. This has been discussed repeatedly as being a pretty big UX win that's highly wanted; I think we should either bite the bullet and call this one of the rate UX-polish blockers, or cut our losses now and punt.

Seems like this isn't too invasive of a change, code-wise, and Frank and I have discussed how to break up the work into contained, bite-size steps such that we can get incremental improvements to tab reordering/detaching as we go. But since it's a sequence, changes some existing UI behaviors, and will have a couple of dependencies (to be filed shortly) we should discuss this up front.
blocking2.0: - → ?
Depends on: 615773
The final patch for his bug will depend on the mouse coordinates being available during drag-and-drop source node events for content to be able to calculate whether the tab will be detached upon releasing the mouse. Thus, I am adding bug 505521 as a dependency. The coordinates only need to be available for those events for chrome code. If there is an easier way to obtain mouse coordinates for drag events, let me know.
Depends on: 505521
I really hope that there's an easier way, because that dependency looks ugly. :/ Enn?
I don't think exposing mouse coordinates for those events is a particularly complex dependency, but Enn would know better. Aside from that, my patch is almost feature-complete now, so once we can resolve that dependency, I could have this up for review in a week.
But what coordinates do you want? Relative to what? Which event?
(In reply to comment #57)
> But what coordinates do you want? Relative to what? Which event?

Mouse coordinates relative to screen left and top (i.e. screenX and screenY) would be easiest to use, but since the positions of the node, window, etc. are already known, a different origin would work too. I'd need it for the "drag" event. (Coordinates are available for "dragover", but that's not helpful when the drag moves outside of the window.)
I have a patch of the detach animation working well enough using Enn's current patch for bug 533460. Will upload with more information once I clean it up.
Depends on: 533460
No longer depends on: 505521, 615773
Attached patch WIP v2 (obsolete) — Splinter Review
This uses the prototype patch from bug 533460, so apply that patch first.
It is *feature-complete* and absolutely dogfoodable.
I'll link to tryserver builds once they're up.
The patch might look large, but a lot of it is just moving or deleting code. 

Remaining known issues:
- There shouldn't be a small unfilled rectangle next to the mouse cursor during drags. This can be fixed with a small change to the patch for bug 533460.
- Dragging a tab over the bookmarks menu button either shouldn't open the menu or should keep the drag image above the menu.
- Dragging a tab over a tab strip scroll button should scroll the tab strip.

More eye candy that we could do in the future in followup bugs:
- Smoothly animate the movement of background tabs.
- Dragging an app tab to the normal tab region or dragging a normal tab to the app tab region moves the dragged tab instead of using the drop indicator.
- Detaching a tab animates the drag image out of the tab strip.
- The detachment threshold is only active when detaching a tab and not when reattaching it.
- Dragging a tab to another window animates the drag image into the tab strip instead of using the drop indicator.
- Dragging a tab to the right of its sibling tabs and the new tab button continues to move the tab within the bounds of the tab strip.
Attachment #492890 - Attachment is obsolete: true
Tryserver builds: https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5f971390c796/

Addenda:

Known issues:
- The drop location of a tab should be dependent upon the element's location not the mouse location.

In the above eye candy section, the statements refer to desired behavior, not current behavior.
Doesn't seem to work on ubuntu, can't move tabs at all. The only thing that happens is some icon appears briefly near the mouse pointer when I try to move a tab.
Will there be any Windows builds? I want to test this feature on Win7..
(In reply to comment #63)
> Will there be any Windows builds? I want to test this feature on Win7..

They are in the previously provided link....https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5f971390c796/tryserver-win32-debug/
That's the debug build...
While awesome, this is a new feature. We'd like to ship Firefox 4 before the singularity occurs. Blocking-.
blocking2.0: ? → -
(In reply to comment #66)
> While awesome, this is a new feature. We'd like to ship Firefox 4 before the
> singularity occurs. Blocking-.

Comment 41 and comment 43 infer that this will make it into Firefox 4 despite the feature freeze?
If the patch is ready and safe, we'd take it. The blocking status says that we won't hold back the release for it if it's not ready.
Ah ok, thanks for the clarification.
Attached patch WIP v3 (obsolete) — Splinter Review
Fixed all known issues within the scope of _this_ bug, including the pause after rearranging tabs.

The two following issues should be fixed in their respective bugs:
- There shouldn't be a small unfilled rectangle next to the mouse cursor during
drags. This can be fixed in bug 533460.
- Dragging a tab over the bookmarks menu button either shouldn't open the menu
or should keep the drag image panel above the menu (could also be fixed in bug 533460).

I'll push this to tryserver when I'm confident that it's ready for a first review.
Attachment #495767 - Attachment is obsolete: true
Attachment #495804 - Flags: feedback?(dolske)
(In reply to comment #63)
> Will there be any Windows builds? I want to test this feature on Win7..

Tryserver isn't instantaneous. The Windows builds with WIP v2 are up now.

(In reply to comment #65)
> That's the debug build...

Debug builds run just fine for trying out code and features.

(In reply to comment #62)
> Doesn't seem to work on ubuntu, can't move tabs at all. The only thing that
> happens is some icon appears briefly near the mouse pointer when I try to move
> a tab.

Neil explained in bug 533460 that his proof-of-concept does not work on Linux.
I'll ask him about it.
In the meantime, I'll code in a blank drag image for Linux.
Blocks: 563540
> Debug builds run just fine for trying out code and features.

If you can run debug builds, yes. (I don't think much of the triers can.)
Attached patch WIP v4 (obsolete) — Splinter Review
Attachment #495804 - Attachment is obsolete: true
Attachment #496124 - Flags: review?(gavin.sharp)
Attachment #496124 - Flags: review?(dao)
Attachment #495804 - Flags: feedback?(dolske)
(In reply to comment #73)
> Created attachment 496124 [details] [diff] [review]
> patch v1

The entire patch was refactored. Small fixes here and there.
Despite the size of the patch, a lot of it is code removal.
It uses a black drag image on Linux, since the patch for bug 533460 has yet to support Linux.
Apply the patch for bug 533460 first.
I'll post tryserver links once available.
Thank you for the test builds! Works perfectly on Windows 7 despite one fact... I'm not joking. I really can see it. The tab re-ordering animation is extremely laggy and slow when using Microsoft Arc mouse (especially when moving the tab quickly). When I'm using touchpad with the same browser window with the same set of tabs, animation is nice and smooth. The CPU usage seems to be similar in both cases, IntelliPoint is not installed.

If you'll decide to take it serious, I'll be happy to try to help with debugging.
Whiteboard: [parity-safari][parity-chrome][parity-ie] → [parity-safari][parity-chrome][parity-ie][target-next-beta]
Whiteboard: [parity-safari][parity-chrome][parity-ie][target-next-beta] → [parity-safari][parity-chrome][parity-ie][target-betaN]
Attached patch WIP v5 (obsolete) — Splinter Review
Attachment #496124 - Attachment is obsolete: true
Attachment #497424 - Flags: review?(gavin.sharp)
Attachment #497424 - Flags: review?(dao)
Attachment #496124 - Flags: review?(gavin.sharp)
Attachment #496124 - Flags: review?(dao)
Comment on attachment 497424 [details] [diff] [review]
WIP v5

Bouncing from gavin to dao... Dao, could you at least a first-pass review done on this, so we can get a better sense of any issues remaining here?
Attachment #497424 - Flags: review?(gavin.sharp)
Comment on attachment 497424 [details] [diff] [review]
WIP v5

Couple of superficial comments from a quick skim through... I'll start a real review if dao doesn't have time.

>+++ b/browser/base/content/tabbrowser.css
...
>+.tab-drag-image {
>+  -moz-transform: translate(1px);
>+}


IIRC, you said this was a workaround to avoid dispatching events to the drag-image panel? This is worth a comment in the CSS, and reference to a bug number if applicable.

>+      <method name="_updateTabDetachState">
...
>+          let bo = this.mTabstrip.boxObject;

How about "box"?
> >+      <method name="_updateTabDetachState">
> ...
> >+          let bo = this.mTabstrip.boxObject;
> 
> How about "box"?

I just took that from:

-        if (eX > wX && eX < (wX + window.outerWidth)) {
-          let bo = this.mTabstrip.boxObject;

but I can definitely change it. :)

I'll address these comments in the next patch once this gets a more thorough review (to avoid excessive versions).
(In reply to comment #68)
> If the patch is ready and safe, we'd take it. The blocking status says that we
> won't hold back the release for it if it's not ready.

So, if this is not implemented in time for Firefox 4 official release (I'm talking NON Release Candidates), will you then aim for the first official patch (say 4.0.1)?

I would really like to see this feature implemented by the time Firefox 4 is out of beta status, because I am not the only one who moves tabs around in full-screen mode, and with the 1/8/11 build there is now no way to tell where tabs are moving to since the tabs don't animate over and the black arrow is invisible in full screen mode.
If this isn't able to be implemented by release, bug 572160 would have to be backed out.
(In reply to comment #81)
> If this isn't able to be implemented by release, bug 572160 would have to be
> backed out.

This is nonsense. The drop indicator needs to be fixed regardless of this bug.
So this bug will block final Firefox 4?
He said that the drop indicators have to be fixed - REGARDLESS of this bug here. So why should it block?
o...  it seems that I misunderstood what he said. my bad. Sorry.
I was trying the patch, it looks good but it's a bit buggy.

1. looks like you completely killed the possibility to drag a tab to bookmarks toolbar or bookmarks menu). Is there a specific reason for this? Unfortunately I use this gesture a lot :(
2. when dragging a tab across windows, often the old arrow indicator appears instead of the new tab. Sometimes the new tab appears but on release a new window is spawned instead of adding the tab.
(In reply to comment #86)
> I was trying the patch, it looks good but it's a bit buggy.
> 
> 1. looks like you completely killed the possibility to drag a tab to bookmarks
> toolbar or bookmarks menu). Is there a specific reason for this? Unfortunately
> I use this gesture a lot :(

I killed this, because once tabs become something that the user is visually dragging, the tab should no longer be overloaded as an affordance that represents the tab's current page. For example, dropping the tab on a web page's text field shouldn't insert the tab's current URL in that field; rather, it should have visually indicated that it would be detached and then be detached upon drop.

The location bar favicon's visual promixity to the URL and hierarchical association with the current location makes it an affordance that represents well the current page. Dragging the location bar favicon naturally inserts the current page, whereever it is dropped. Having the favicon inside the identity block does muddle this association, but we are moving the favicon outside of the identity block (bug 610048).

> 2. when dragging a tab across windows, often the old arrow indicator appears
> instead of the new tab.

I'm leaving this animation (replacing the drop indicator with a tab when dragging tabs between windows) to a followup, since it makes this bug much more complicated and dragging between windows is not the common case.

> Sometimes the new tab appears but on release a new
> window is spawned instead of adding the tab.

I don't see this. STR?

With the revised schedule for Firefox 4 and amount of work in both my, Dao's, and Neil's hands, I don't think it makes sense to continue to push for getting this into Firefox 4. It will be one of the first things to tackle for the subsequent release.
Attachment #497424 - Flags: review?(dao)
(In reply to comment #87)
> With the revised schedule for Firefox 4 and amount of work in both my, Dao's,
> and Neil's hands, I don't think it makes sense to continue to push for getting
> this into Firefox 4. It will be one of the first things to tackle for the
> subsequent release.

So, do you mean the users won't see tab movement animations in Firefox (like is already available in IE 9 Beta) until Firefox 5.0 or within one of the first official patches for 4.0?
I really feel this should be a blocking bug for Firefox 4, because I can't be the only one who moves tabs around in full screen and can't tell where I am moving them to.  Before you put tabs in line with the menu bar in the 1/8/11 Minefield build, this was never an issue because the tabs always remained below the menu bar in full and non-full screen modes.
(In reply to comment #88)

> So, do you mean the users won't see tab movement animations in Firefox (like is
> already available in IE 9 Beta) until Firefox 5.0 or within one of the first
> official patches for 4.0?

It won't be in 4.0 -- that's the right call, there's a ton of awesomeness in 4.0 compared to 3.6 and we need to ship. What release this patch hits is something we can consider after 4.0 ships.
All I want is some indication in full-screen when moving tabs.  The black arrow
is now hidden when in full-screen because it is set to be drawn at horizontal
position 0, which can't be seen if the tabs are ALSO at position 0.
(In reply to comment #91)
> All I want is some indication in full-screen when moving tabs.  The black arrow
> is now hidden when in full-screen because it is set to be drawn at horizontal
> position 0, which can't be seen if the tabs are ALSO at position 0.

That's being handled by a separate bug.
(In reply to comment #92)
> (In reply to comment #91)
> > All I want is some indication in full-screen when moving tabs.  The black arrow
> > is now hidden when in full-screen because it is set to be drawn at horizontal
> > position 0, which can't be seen if the tabs are ALSO at position 0.
> 
> That's being handled by a separate bug.

Right, bug 572160, which is listed as blocking.
(In reply to comment #87)
> The location bar favicon's visual promixity to the URL and hierarchical
> association with the current location makes it an affordance that represents
> well the current page.

So, the suggestion is to switch to the tab I want to drag, and drag the favicon instead. Sure, this works, it's just a bit longer (if I want to drag a inactive tab) and probably far from what a user will try to do at first, but it's just matter of getting used to it, as many other changes.
Mostly I was guessing if there were also technical reasons to disable drop to bookmarks, or was just a design decision.

> > Sometimes the new tab appears but on release a new
> > window is spawned instead of adding the tab.
> 
> I don't see this. STR?

Well, sometimes it worked correctly, sometimes not, so it's hard to give steps :( Mostly I was trying to drop a tab from window A to the first position in window B, and about 1/4 of the times the tab was detached rather than dropped in that window.
I can understand this is hard work, but this is something you use everyday, dozens of times – and I think it’s a significant drawback in polish that makes firefox look outdated and clumsy.

If not for 4.0 I hope this will be in 4.0.1 or another minor update, Firefox 4.5 a year later.
(In reply to comment #94)
> (In reply to comment #87)
> > The location bar favicon's visual promixity to the URL and hierarchical
> > association with the current location makes it an affordance that represents
> > well the current page.
> 
> So, the suggestion is to switch to the tab I want to drag, and drag the favicon
> instead. Sure, this works, it's just a bit longer (if I want to drag a inactive
> tab) and probably far from what a user will try to do at first, but it's just
> matter of getting used to it, as many other changes.
> Mostly I was guessing if there were also technical reasons to disable drop to
> bookmarks, or was just a design decision.

A design decision. Since to the user, the tab has already visually moved out of the tab strip and become the unit that the user is dragging, it no longer makes as much sense to allow dropping the current URL on any target that will take it. It produces way too much drop targets, which the user has to avoid when wanting to detach a tab, and also looks odd to bring back the tab in the tab strip after having dropped it into, say, a bookmarks folder.
A simple example of how this could go wrong:
1. The user begins to drag a tab downward (tabs-on-top).
2. Approximately somewhere vertically within the location bar, the tab visually indicates that it will be detached upon release and is no longer within the tab strip.
3. If the user releases the tab over the location bar, the browser will load that tab's current page in the current tab. (When dragging the current tab, this effectively reloads the page.) Then the tab reappears in the tab strip.
4. If the user releases the tab over the bookmarks toolbar, a bookmark will be created. Then the tab reappears in the tab strip.
5. If the user releases the tab in a web page text field, the tab's current URL will be inserted into the text field. The tab then reappears in the tab strip.
If we allow the tab to be an affordance to drag the page URL, the user must avoid all these elements when attempting to detach a tab. If we want to indicate precisely that the tab's URL will be dropped and not the tab itself, it would result in visual jerkiness as the tab disappears and reappears in the tab strip or morphs shape during the drag.
(In reply to comment #87)
> I killed this, because once tabs become something that the user is visually
> dragging, the tab should no longer be overloaded as an affordance that
> represents the tab's current page.

I disagree and think this is a mistake. Adding this functionality shouldn't remove other functionality. The user shouldn't and generally doesn't care about the mechanisms behind the scenes. The ability to grab a tab and either reorder them or drag it to the bookmarks menu or linkbar shouldn't be mutually exclusive, especially since this isn't adding the reorder ability, but modifying that.

If I grab a desktop icon, I can drag it to another position (reorder), drag it into a folder (reorder/move, akin to moving between windows, or bookmarking), or drag it to the system's recycle bin/trash (delete), or if it's a file icon as opposed to an application icon, I drag the file onto an application icon, and open that file in that application. I can perform multiple types actions with the same user action, I'm not artificially limited.

In Windows 7, the location bar in Explorer serves multiple purposes. Itshows my current location and the hierarchy to that location; each branch in the location tree is both a quick "go to" button, and a flyout list for navigating; the entire location is also editable text like a command line. Simply because it's a graphical element didn't mean the developers had to artificially limit it's functionality.

As far as the user is concerned, the tab is an object representing a page, and they want to be able to reorder that page, bookmark that page, create a button on the linkbar, heck, I can't even see why they shouldn't be able to drag the tab to the PRINT button if they have it on the toolbar.

Once you make an object dragable, you need to be prepared to create interactions that the user expects, not retrain the user because you dislike the metaphor. Users think, "this will go where I drag it." When you put a paper in a file, it's filed; when you put it in a shredder, it's shredded; when you put it on your desk, it stays there. You don't need to change the mode of the paper depending where its going. 

> For example, dropping the tab on a web
> page's text field shouldn't insert the tab's current URL in that field; rather,
> it should have visually indicated that it would be detached and then be
> detached upon drop.

Why can't it do both? I grant you replacing the current URL in the URLbar with the current URL is pointless, but the concept that the dragable object interacts with whatever object it is being dragged to in the manner the destination object operates is more intuitive than saying "To interact in method A, drag page-object 1. To interact in method B, drag page-object B."
(In reply to comment #97)
> The user shouldn't and generally doesn't care about
> the mechanisms behind the scenes.

This has nothing to do with implementation details. It's a design decision.

> If I grab a desktop icon, I can drag it to another position (reorder), drag it
> into a folder (reorder/move, akin to moving between windows, or bookmarking),
> or drag it to the system's recycle bin/trash (delete), or if it's a file icon
> as opposed to an application icon, I drag the file onto an application icon,
> and open that file in that application. I can perform multiple types actions
> with the same user action, I'm not artificially limited.

That works fine, because you're moving a ghosted drag image of the icon, not the icon itself. We move a ghosted tab drag image right now in Firefox 3.6. What we want to do is move the tab itself.
My comment is in regards to "my prototype (mirror of link in comment 12)".

This is exactly how it should work.  However, the tabs should not be able to be dragged past the left and right edges of the screen when ready to be implemented in the Firefox code.
(In reply to comment #98)
> This has nothing to do with implementation details. It's a design decision.

I understand, but that weakens your argument.

> That works fine, because you're moving a ghosted drag image of the icon, not
> the icon itself. We move a ghosted tab drag image right now in Firefox 3.6.
> What we want to do is move the tab itself.

I don't think that matters. If they drop it on the bookmarks bar or menu, then the tab can snap back to the previous position, and then the menu or bar will behave as it does in Fx3.x.
Did I read right, users will no longer be able to drag tabs in order to bookmark them? Neither to the bookmarks toolbar nor the bookmarks?
(In reply to comment #98)
> That works fine, because you're moving a ghosted drag image of the icon, not
> the icon itself. We move a ghosted tab drag image right now in Firefox 3.6.
> What we want to do is move the tab itself.

I don't see the problem. When moving the tab in the tab bar just move the tab. When moving it out of the tab bar convert it to a ghost image and leave the original tab in the bar. When entering a tab bar again remove the original tab and insert the tab you're dragging at the new location and drag it there.
+1 at comment #95. This and the "tabs on the title bar" are the two major features I expect and want for FF4. FF4 is already way behind other modern browsers in terms of UI, so I won't be happy if this gets pushed to the next release..
Blocks: 635355
So can this bug get some attention now that Firefox 4's been released?
I am using this patch for a few weeks now. I really like this approach to reorder tabs, but I noticed a few flaws:

1.) You can accidentally trigger the tear off. It is quite clear when you tear off a tab vertically (it disappears form the tab bar). But if you reorganize tabs and you let a tab go before it is perfectly in its new position you trigger a tear off. IMHO it would be better if it snaps to the indicated new position.

2.) If you've torn off a tab you have no indication what so ever that the tab is where your mouse pointer is. In the fx4 release there is a small picture of the tab. I think with this new animation the tab should stick to the pointer.

3.) When reattaching a tab, there is only a small arrow indicator. I think it would be nice if this activates the animation, too.
(In reply to comment #106)

Thanks for your feedback. On what platform are you running?

Also, this patch is still an incomplete work-in-progress, and I'll be updating it within the week.
Attachment #497424 - Attachment description: patch v2 (minor and typo fixes) → WIP v2 (minor and typo fixes)
I’m using Win 7 Professional 64bit.
Thanks for all your work, I am looking forward to the updated patch.
Attached patch WIP v6 (obsolete) — Splinter Review
Unbitrotted and updated to use the latest patch from bug 533460
( https://bug533460.bugzilla.mozilla.org/attachment.cgi?id=520652 )

Pushed to tryserver:
http://tbpl.mozilla.org/?tree=MozillaTry&pusher=fyan@mozilla.com&rev=2886cf2446e0
Will test on Windows and Linux when builds are available.
Attachment #497424 - Attachment is obsolete: true
Whiteboard: [parity-safari][parity-chrome][parity-ie][target-betaN] → [parity-safari][parity-chrome][parity-ie]
(In reply to comment #109)
> Created attachment 524959 [details] [diff] [review]
> WIP v3
Works very nice. But dragging tab to bookmarks toolbar doesn't bookmark them.
(In reply to comment #110)
> (In reply to comment #109)
> > Created attachment 524959 [details] [diff] [review]
> > WIP v3
> Works very nice. But dragging tab to bookmarks toolbar doesn't bookmark them.

as previously discussed this will be the wanted behavior, once tabs become more like "window" entities, doesn't make sense to add a window to a toolbar. You can still drag the favicon from the locationbar, usually you will drag the tab for the current visible page, so that should not be any harder.
In this day and age where touch-screen interfaces will soon be the norm I can see this making a return at the some point in the future. But if it's a trade-off between a modern day feel to tab rearrangement along the tab bar and bookmarking functionality, I'll happily retrain myself to use the star more. Hopefully functionality will be improved in that regards. Bug 582907 deals with that, though not to a degree that it would block this bug.
Yes, we should probably tweak the star button functionality, I discussed some change with Faaborg at the last all-hands about that, no schedule for now, but I feel like this bug has higher priority than a tab-toolbar drag gesture today.
Just played with the latest try build. Love the tear-off, and it feels really well done and nippy. 

I do have a couple points/questions to raise however:
1/ If you tear off a tab and hover of the bookmarks menu (on the bookmarks bar) should it open the bookmarks menu? I would think that the intended behaviour would be to ignore the menu since you can't drop contents there.

2/ Upon dragging a tab to App Tabs it turns into a drop marker. Is this intended? It seems inconsistent? Especially since you can't create App Tabs if you have none placed in this manner. Also if we were going to take this approach, wouldn't it be best to simply shrink the tab upon reaching the App Tab zone?

3/ The styling of a tab when it turns into a Window seems out of place, would it for example be possible to create a glass holder on Windows 7?

4/ If you have a window full of tabs and then tear off one of those tabs, you have a window with a single tab. At that point there tear off threshold disappears, is this correct? However the issue is that the window stays in place, shouldn't it disappear as you have it in your hand/cursor. On a Window full of tabs, the tab disappears from the tab bar the second you start to tear it for example. It seems to me the current implementation requires the user to hover over the icon to get back to the old window and drop the tab there. That seems overly complex for some users.
(In reply to comment #114)
> Just played with the latest try build. Love the tear-off, and it feels really
> well done and nippy. 
> 
> I do have a couple points/questions to raise however:
> 1/ If you tear off a tab and hover of the bookmarks menu (on the bookmarks bar)
> should it open the bookmarks menu? I would think that the intended behaviour
> would be to ignore the menu since you can't drop contents there.

Sounds like a bug. This is handled by http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js, it should actually check if the datatransfer has acceptable data type before opening the menu, looks like it is not doing that.
Dragging is awesome, but the detaching / attaching back is little bit buggy (flickerings and the other tabs don't slide when detaching but jump... ) and too primitive. The prototype (attachment 463283 [details])  would be nice for detaching.
(In reply to comment #114)
> 2/ Upon dragging a tab to App Tabs it turns into a drop marker. Is this
> intended? It seems inconsistent? Especially since you can't create App Tabs if
> you have none placed in this manner. Also if we were going to take this
> approach, wouldn't it be best to simply shrink the tab upon reaching the App
> Tab zone?
I'd prefer the way it is.

> 3/ The styling of a tab when it turns into a Window seems out of place, would
> it for example be possible to create a glass holder on Windows 7?
Can be done in a follow up.
How to try this on Windows ?
You need to install the try-server build. You can find it here :
[url]http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/[/url]

I agree with [url=https://bugzilla.mozilla.org/show_bug.cgi?id=455694#c117]Comment 117[/url] detaching should be improved one way or another.
Otherwise it seems pretty good overall.
I applaud the UI team for working on this somewhat complex new feature so that the Firefox browser can become more modern and be in line with IE, Safari and Chrome (which already has this feature).  This requires a lot of code and polishing to iron out all issues / bugs.

However, this feature has been suggested 2.5 years ago and still isn't quite ready yet.

Does anyone have an estimated target date for these features?

It was supposed to be added to Firefox 4, but what is the time frame now?

Firefox 5, 6.....even later than that?  I don't see any target / blocking numbers above.
No earlier than Fx6. The code for Fx5 was already merged over to Aurora, so that ship has sailed.
(In reply to comment #114)
> 1/ If you tear off a tab and hover of the bookmarks menu (on the bookmarks bar)
> should it open the bookmarks menu? I would think that the intended behaviour
> would be to ignore the menu since you can't drop contents there.

(In reply to comment #116)
> better link:
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#849

I had been trying to figure out how to fix that. Thanks for the pointer, Marco. :)

(In reply to comment #114)
> 2/ Upon dragging a tab to App Tabs it turns into a drop marker. Is this
> intended?

It is intended for now, simply because fixing that is difficult, and I'd rather get this landed first. It will be handled in followup bug 579629.

> 3/ The styling of a tab when it turns into a Window seems out of place, would
> it for example be possible to create a glass holder on Windows 7?

I'm aware that the styling of the tab is very basic at the moment. Again, this can be done in a followup.

> 4/ If you have a window full of tabs and then tear off one of those tabs, you
> have a window with a single tab. At that point there tear off threshold
> disappears, is this correct? However the issue is that the window stays in
> place, shouldn't it disappear as you have it in your hand/cursor.

No. One of the key points is that it should be easy to reattach during the drag. The threshold and detach animation could definitely be tweaked, but I'll leave that detail to a followup.

(In reply to comment #117)
> Dragging is awesome, but the detaching / attaching back is little bit buggy
> (flickerings and the other tabs don't slide when detaching but jump... ) and
> too primitive. The prototype (attachment 463283 [details])  would be nice for detaching.

I am aware.

(In reply to comment #120)
> You need to install the try-server build.

> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/

Yup, that's the best build to try it so far. I haven't been advertising the tryserver builds just yet, because I haven't done enough testing on Windows and Linux. I estimate that by Monday, the patch will be ready for review, and I'll link a new tryserver build by then.
> (In reply to comment #116)
> > better link:
> > http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#849
> 
> I had been trying to figure out how to fix that.

Well, the tricky part is that you should walk all the dataTransfer (since I could be dragging multiple elements) and see if some of the data is not accepted (iirc if even just 1 data is not accepted we don't allow the drop).
The good news is that probably you can use PlacesControllerDragHelper.canDrop (something similar to what we do here http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#914)
This code is still a bit hairy, so in case feel free to drop the issue to a follow-up, it should not block the feature from landing.
(In reply to comment #124)
> (In reply to comment #114)
> (In reply to comment #117)
> > Dragging is awesome, but the detaching / attaching back is little bit buggy
> > (flickerings and the other tabs don't slide when detaching but jump... ) and
> > too primitive. The prototype (attachment 463283 [details])  would be nice for detaching.
> 
> I am aware.

No offense intended. Are you planning to add some behaviour like in the prototype?
Attachment #496124 - Attachment description: patch v1 → WIP v4
Attachment #497424 - Attachment description: WIP v2 (minor and typo fixes) → WIP v5
Attachment #524959 - Attachment description: WIP v3 → WIP v6
Attached patch patch (obsolete) — Splinter Review
The patch is ready for review now, AFAICT.

Windows and OS X users can try out these builds:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/

The patch's code has been slightly modified since that build was made, but the behavior didn't really change, IIRC.

Stuff is still broken on Linux, but that's due to the patch for bug 533460 being incomplete.
Attachment #524959 - Attachment is obsolete: true
Attachment #526442 - Flags: ui-review?(limi)
Attachment #526442 - Flags: review?(dolske)
Attached patch addendum workaround for Linux (obsolete) — Splinter Review
As I said, the drag panel that appears when tabs are detached still doesn't seem to work on Linux, so we might need this #ifdef workaround for Linux, if we can't fix it in time.

LINUX, Y U NO WORK :(
What doesn't work about it in Linux?
Could be this is related to the longstanding drag bug Places has on Linux?
For us, in Linux, as soon as a popup opens, it's impossible to start a drag (for example folders in the toolbar, they work because we try to figure out the will of the user based on previous mouse movements).
(In reply to comment #129)
> What doesn't work about it in Linux?

Using this build:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/tryserver-linux/

When I start dragging a tab, the drag panel appears very briefly and then disappears, which also internally cancels the drag operation, so I cannot move the tab.

The expected behavior (and actual behavior on Windows and OS X) is that the panel is open but invisible due to opacity:0 until dragging such that the tab would be detached at which point opacity:1 is set so that the panel is visible.

(In reply to comment #130)
> Could be this is related to the longstanding drag bug Places has on Linux?
> For us, in Linux, as soon as a popup opens, it's impossible to start a drag
> (for example folders in the toolbar, they work because we try to figure out the
> will of the user based on previous mouse movements).

That might be it...
Actually, even with the #ifdef workaround, the drag image still disappears like that, with no errors in the Error Console as evidence. Now I'm not sure what the problem is. x(
(In reply to comment #132)
> Actually, even with the #ifdef workaround, the drag image still disappears like
> that, with no errors in the Error Console as evidence. Now I'm not sure what
> the problem is. x(

Oops, I was using XP_LINUX, which isn't defined in that file.

This fixed patch addendum demonstrates that the panel is not working and actually breaking the drag on Linux.
Attachment #526446 - Attachment is obsolete: true
To clarify, the above addendum resets the drag image to a blank canvas, instead of using the panel, since my attempts to use the panel have so far only resulted in the drag operation being aborted immediately after dragstart.
I have tried the patch and it works nice, great work! There is one thing that I think should be solve: when you drag the last tab of a window, the window should dissapear during the dragging process, allowing you to drag its tab to a windows which is just behind.
yeah , just new tab button doesnt look good and is illogical too
(In reply to comment #135)
> I have tried the patch and it works nice, great work! There is one thing that I
> think should be solve: when you drag the last tab of a window, the window
> should dissapear during the dragging process, allowing you to drag its tab to a
> windows which is just behind.
Chrome 12 already does this. This should be implemented for aero snap (Bug 563540) as well.
(In reply to comment #135)
(In reply to comment #136)
(In reply to comment #137)

Patches are welcome.

(In reply to comment #125)
> Well, the tricky part is that you should walk all the dataTransfer (since I
> could be dragging multiple elements) and see if some of the data is not
> accepted ...
> This code is still a bit hairy, so in case feel free to drop the issue to a
> follow-up, it should not block the feature from landing.

Thanks for the pointers. I think this patch already has enough complexity. I'll leave that a followup, which perhaps you would be better suited to take. ;)
(In reply to comment #135)
> I have tried the patch and it works nice, great work! There is one thing that I
> think should be solve: when you drag the last tab of a window, the window
> should dissapear during the dragging process, allowing you to drag its tab to a
> windows which is just behind.

see Bug 597989
(In reply to comment #131)
> (In reply to comment #129)
> > What doesn't work about it in Linux?
> 
> Using this build:
> https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fyan@mozilla.com-5d705f7193ee/tryserver-linux/
> 
> When I start dragging a tab, the drag panel appears very briefly and then
> disappears, which also internally cancels the drag operation, so I cannot move
> the tab.

I investigated the problem further, and it seems that immediately after a property is set that would cause the panel to be rendered (e.g. panel.hidden = false;), the above happens (appear, disappear, drag cancelled).

I haven't been able to find a solution or workaround besides using a blank canvas instead of the panel on Linux for now, which I think is acceptable, since Linux has a special cursor for dragging, which mitigates the detach feedback problem.
Attached patch Places changes (obsolete) — Splinter Review
Attachment #528244 - Flags: review?(sdwilsh)
Attached patch patch v2 (obsolete) — Splinter Review
Optimized tab positioning code.
Polished dragging between app tab and normal tab regions.
Attachment #526442 - Attachment is obsolete: true
Attachment #528246 - Flags: ui-review?(limi)
Attachment #528246 - Flags: review?(dolske)
Attachment #526442 - Flags: ui-review?(limi)
Attachment #526442 - Flags: review?(dolske)
Attachment #526467 - Attachment is obsolete: true
Attachment #528247 - Flags: review?(dolske)
Comment on attachment 528244 [details] [diff] [review]
Places changes

>diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js

>       // Thus we shouldn't use unwrapNodes here at all if possible.
>       // I think it would be OK to accept bogus data here (e.g. text which was
>       // somehow wrapped as TAB_DROP_TYPE, this is not in our control, and
>       // will just case the actual drop to be a no-op), and only rule out valid
>       // expected cases, which are either unsupported flavors, or items which
>       // cannot be dropped in the current insertionpoint. The last case will
>       // likely force us to use unwrapNodes for the private data types of
>       // places.
>-      if (flavor == TAB_DROP_TYPE)
>-        continue;
>-

you didn't fix the above comment that talks about TAB_DROP_TYPE.

also, what discussed in comment 115 appears not addressed (nor a follow-up has been filed).
Attachment #528244 - Flags: review?(sdwilsh) → review-
Attached patch Places changes v2 (obsolete) — Splinter Review
(In reply to comment #144)

Thanks for the review. :)

> you didn't fix the above comment that talks about TAB_DROP_TYPE.

Fixed.

> also, what discussed in comment 115 appears not addressed (nor a follow-up has
> been filed).

Addressed and fixed, AFAICT.
Attachment #528244 - Attachment is obsolete: true
Attachment #528455 - Flags: review?(mak77)
Comment on attachment 528455 [details] [diff] [review]
Places changes v2

Review of attachment 528455 [details] [diff] [review]:

r=me with these changes, assuming you manually tested that dragging a tab does not open the menu, while dragging a bookmark from the toolbar and the favicon from the locationbar still open the menu correctly.

::: browser/base/content/browser-places.js
@@ +850,5 @@
     // Opening menus in a Places popup is handled by the view itself.
     if (!this._isStaticContainer(event.target))
       return;
 
+    if (PlacesControllerDragHelper.canDrop(null, event.dataTransfer)) {

Here you should generate an insertion point to the end of the bookmarks menu,

let ip = new InsertionPoint(PlacesUtils.bookmarksMenuFolderId,
                            PlacesUtils.bookmarks.DEFAULT_INDEX,
                            Ci.nsITreeView.DROP_ON);
and pass it as the first argument to canDrop

::: browser/components/places/content/controller.js
@@ +1369,5 @@
         return false;
       }
 
+      if (!ip) // We might not have an insertion point yet (e.g. onDragEnter).
+        continue;

I suppose you did this because you were passing a null ip, I prefer if you pass a guessed ip to the bookmarks menu, as said above, and avoid this change.
Attachment #528455 - Flags: review?(mak77) → review+
(In reply to comment #146)
> r=me with these changes, assuming you manually tested that dragging a tab does
> not open the menu, while dragging a bookmark from the toolbar and the favicon
> from the locationbar still open the menu correctly.

Yes, I tested it :)

> PlacesUtils.bookmarks.DEFAULT_INDEX,

I misunderstood what DEFAULT_INDEX meant, so I thought it couldn't be used, but I get it now. Thanks for the review!
Attachment #528455 - Attachment is obsolete: true
Try builds expire, so here's a (semi)permalink to builds with this patch (and some other tab-related patches):

https://people.mozilla.com/~fyan/try-builds/tab-animations/
I think that Moving Animation is too slow while you move tabs fast. It looks like kind of "animation freeze" for a few first moments of animation.
Blocks: 566510
I stumbled across a problem:

If a loading tab triggers the master password dialog while you are dragging a tab, the whole window freezes, nothing is clickable and you eventually have to kill the firefox process.
I use a single core processor Intel Celeron. When you try to move the tabs, the CPU loaded at 100%. What causes it impossible to freezes.
I want to note that a similar animation in Chrome, Opera or IE working smoothly and seamlessly.
I found something : You could call it a bug or a missing feature.

When we have 2 windows of firefox open. If we detach one tab from one of the window(say window 1) and drag it to the tab strip of the other window(say windows 2), the tab from the tab strip of window 1 is removed , but no tab is added to the tab strip of window 2. Just the arrow is shown. 

What is required would be something like this : 
we detach a tab from window 1 , the tab from the tab strip of Window 1 immediately disappears. And when we add that tab to Window 2, its tab should be added to Window 2's tab strip along with the tab rearrangement animation in action.

I hope my idea is clear to all and I wish to see this feature in this animation.
Blocks: 654907
Blocks: 590297, 579482, 579629
No longer blocks: 579482
(In reply to comment #148)
> Try builds expire, so here's a (semi)permalink to builds with this patch
> (and some other tab-related patches):
> 
> https://people.mozilla.com/~fyan/try-builds/tab-animations/

Good job Frank! Is in intended that there is no animation when moving normal tabs into app tabs area?
blocking2.0: - → ?
blocking2.0: ? → ---
Comment on attachment 528246 [details] [diff] [review]
patch v2

Review of attachment 528246 [details] [diff] [review]:
-----------------------------------------------------------------

This is basically r+, but I'm just going to clear review instead, since there are a bunch of nits and a few questions. Will r+ an updated version, assuming all else turns out well! \o/

::: browser/base/content/browser.css
@@ +80,5 @@
> +
> +.tabbrowser-tabs[drag=move] > .tabbrowser-tab:not([selected]) {
> +  -moz-transition: max-width 1ms, -moz-transform 200ms ease-out;
> +}
> +

So, AIUI (from talking with Frank), this 1ms transition on max-width is just to ensure we always get a transitionend event -- specifically, when a tab closes while dragging it (since we currently hook transitionend to finish tab-close stuff). It's otherwise irrelevant to the feature this patch is implementing.

I really hate how CSS transitions work in this respect... Since CSS only applies 1 rule, you can't have independent implementations of things using -moz-transition for the same node. [Just as "color: black" and "color: white" doesn't give you grey. But this is more akin to deconflicting "border: X" and "border: Y" by having separate "border-color: X" and "border-style: Y" properties.]

So, definitely add a comment here, and we should talk with dbaron if there's something that can be done with the spec to help in the long run.

::: browser/base/content/tabbrowser.xml
@@ +1663,5 @@
>                      tabs._scrolledToEnd = false;
>                  }, 0, this.tabContainer);
>                }
> +
> +              this.tabContainer._finishTabDrag(aTab);

Just FYI, this chunk no longer applies cleanly on trunk.

@@ +2660,5 @@
>  
>      <content>
>        <xul:hbox align="end">
> +        <xul:panel type="drag-image" allowevents="false" hidden="true"
> +                   class="tab-drag-image" anonid="tab-drag-image"/>

Dunno if it's specifically from this, but on my Windows debug build, when I drag the foreground tab I get an assert in nsMenuPopupFrame.cpp:1147 about mPrefSize.width/.height not expected to be zero.

It's also super slow and janky (like 1Hz update rate, .gifs freeze up too) while dragging. Not sure what's up with that, something about my debug build, I assume. Hmm, well, my OS X debug build is nice and smooth, though. Have you tried this on a Windows debug build?

@@ +2756,5 @@
>        <field name="_tabDropIndicator">
>          document.getAnonymousElementByAttribute(this, "anonid", "tab-drop-indicator");
>        </field>
>  
> +      <method name="_positionDropIndicator">

As noted when we talked, this function is just a move of the old |dragover| handler, with the trivial addition of the |scrollOnly| arg.

@@ +2853,5 @@
> +        ]]></body>
> +      </method>
> +
> +      <field name="_tabDragImage">
> +        document.getAnonymousElementByAttribute(this, "anonid", "tab-drag-image");

Sigh. I wonder how much time we spend evaluating all the <field>s in this file...

@@ +2861,5 @@
> +        <parameter name="event"/>
> +        <body><![CDATA[
> +          let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0);
> +          if (!draggedTab || !draggedTab.parentNode)
> +            return;

The .parentNode check is just here to make sure the tab hasn't been closed, right?

Maybe add a <property name="isStillOpen" onget="this.parentNode != null"/>?

Self-documenting code FTW. :)

@@ +2864,5 @@
> +          if (!draggedTab || !draggedTab.parentNode)
> +            return;
> +
> +          let parent = draggedTab.parentNode;
> +          if (parent.childNodes.length == 1) {

parent.childElementCount == 1?

@@ +2872,5 @@
> +
> +          let bo = this.mTabstrip.boxObject;
> +          // detach if leaving window or at least a tab height down from tabbar
> +          // Note: we do not have the ability to detect drags over the titlebar
> +          if (event.type == "dragexit" || event.screenY > bo.screenY + 2 * bo.height) {

For future's sake, please expand this comment a tiny bit wrt why the tab height bit is done.

@@ +2881,5 @@
> +              delete draggedTab._dropIndex;
> +            }
> +            // TODO: we should call _blurTab here, but it won't be safe until
> +            // Panorama is properly integrated and behaves nicely
> +            return true;

Please file a bug for this and note it here.

@@ +2888,5 @@
> +          parent.setAttribute("drag", "move");
> +        ]]></body>
> +      </method>
> +
> +      <method name="_handleTabDrag">

_handleTabDragover() would be more accurate.

@@ +2894,5 @@
> +        <body><![CDATA[
> +          let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0);
> +          if (!draggedTab.parentNode) // tab was closed during drag
> +            return;
> +          event.preventDefault();

... // indicate a drop is possible

@@ +2895,5 @@
> +          let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0);
> +          if (!draggedTab.parentNode) // tab was closed during drag
> +            return;
> +          event.preventDefault();
> +          event.stopPropagation();

Is the .stopPropagation really needed here?

@@ +2901,5 @@
> +            return;
> +
> +          let newIndex = this._getDropIndex(event, draggedTab);
> +          let tabAtNewIndex = this.childNodes[newIndex > draggedTab._tPos ?
> +                                              newIndex-1 : newIndex];

|newIndex - 1| (spacing)

@@ +2942,5 @@
> +            else if (tab._tPos > draggedTab._tPos && tab._tPos < newIndex)
> +              tab.style.MozTransform = "translate(" + -tabWidth + "px)";
> +            else
> +              tab.style.MozTransform = "";
> +          });

Couple of comments here:

1) Part of me wonders if it would be better to avoid the -moz-transforms, and instead insert dummy/invisible tab(s), plus controlling their width, in order to make the bits slide around. But I don't see anything fundamentally _wrong_ with the transform, and that would mean rewriting this from scratch. So, fine. :)

2) While playing with this I noticed a bit of glitchyness on OS X... While dragging a tab, sometimes the other tabs would get a :hover hilight applied, and then that other tab flickers back to the non-:hover state when you drop the tab. I think this can be addressed in a followup if you file it, though. :)

@@ +3226,5 @@
> +            case "dragover":
> +              this._handleTabDrag(aEvent);
> +              break;
> +            case "dragexit":
> +              if (aEvent.target == document)

Why check |.target == document|?

Sidenote: our documentation sure is vague about the difference/existence of dragexit vs dragleave.

@@ +3303,5 @@
> +          let tabs = this.tabbrowser.visibleTabs;
> +
> +          if (draggedTab) {
> +            let tabX = draggedTab.boxObject.screenX + draggedTab._dragDistX;
> +            let right = draggedTab._dragDistX > 0;

s/right/[dragging|moving]Right/?

@@ +3313,5 @@
> +                if (compare(eX, tabs[i].boxObject.screenX + tabs[i].boxObject.width / 2, ltr))
> +                  return i;
> +
> +            let i = tabs.indexOf(draggedTab), tab = draggedTab, next;
> +            while (next = ltr ^ right ? tabs[--i] : tabs[++i]) {

Congrats on a rare, legit use of XOR. :)

@@ +3324,5 @@
> +                  x < this.mTabstrip._scrollButtonUp.boxObject.screenX)
> +                break;
> +              tab = next;
> +            }
> +            return tab._tPos + (ltr ^ right ? 0 : 1);

...again. :)

@@ +3330,5 @@
> +
> +          let tab = this._getDragTargetTab(event);
> +          for (let i = tab ? tab._tPos : 0; i < tabs.length; i++)
> +            if (compare(eX, tabs[i].boxObject.screenX + tabs[i].boxObject.width / 2, ltr))
> +              return tabs[i]._tPos;

This loop is subtly different now, in that |tabs| is .visibleTabs and not .childNodes, but that looks to be ok.

@@ +3335,5 @@
> +          return this.childNodes.length;
> +        ]]></body>
> +      </method>
> +
> +      <method name="_moveTab">

This method name is a little confusing, given there's already a _moveTabTo(). What was it we came up with? slideTab()?

@@ +3369,5 @@
> +            if (window.getComputedStyle(this).direction == "rtl")
> +              displacement *= -1;
> +            let transform = "translate(" + displacement + "px)";
> +            let currentTransform = draggedTab.style.MozTransform;
> +            if (currentTransform && currentTransform != transform) {

Add comment on this "caching".

@@ +3377,5 @@
> +                if (event.propertyName != "-moz-transform")
> +                  return;
> +                this.removeEventListener("transitionend", finish, false);
> +                this.parentNode._finishTabDrag(this);
> +                this.parentNode.tabbrowser.moveTabTo(this, newIndex);

Hmm. I suppose this is one of the seemingly few times where an event listener's |this| is a useful value.

Still, I'd posit that it would be worth making this a bit more robust by checking event.eventPhase == Event.AT_TARGET, lest this code ever see a bubbling transitionend from elsewhere. And maybe s/this/draggedTab/, just to avoid the "hey, it's using 
this|, wonder if it's intentional".

@@ +3563,5 @@
> +        if (this.childNodes.length == 1) {
> +          let windows = Services.wm.getEnumerator("navigator:browser"), n = 0;
> +          for (; windows.hasMoreElements(); windows.getNext(), n++);
> +          if (n == 1) // only one tab and one browser window, so nowhere to drop
> +            return;

What if there's another window, but it's a popup window? [ala nsIBrowserGlue's getMostRecentBrowserWindow()].

I suppose there's no point in making this code more complex (and potentially confusing to the user?) for such an edge case, though.

@@ +3574,5 @@
> +
> +        let dragImage = this._tabDragImage;
> +        dragImage.appendChild(tabPreviews.get(tab));
> +        dragImage.hidden = false;
> +        dt.setDragImage(dragImage, -1, -1);

I'm a little confused here. Why are you doing .setDragImage() here, when the panel is also being unhidden?

@@ +3596,5 @@
> +      ]]></handler>
> +
> +      <handler event="drop"><![CDATA[
> +        this._tabDropIndicator.collapsed = true;
> +        event.stopPropagation();

Again with the .stopPropagation()? Oh, I guess this is just moving code. Hmm.

@@ +3610,5 @@
> +        if (draggedTab) {
> +          let parent = draggedTab.parentNode;
> +          if (parent == this) {
> +            // It is better to handle tab moving on drop when possible,
> +            // since there is a lag between the drop and dragend events.

Did you investigate and/or file a bug on the apparent delay?

@@ +3636,5 @@
>  
>            // We need to select the tab after we've done
>            // swapBrowsersAndCloseOther, so that the updateCurrentBrowser
>            // it triggers will correctly update our URL bar.
> +          this.selectedItem = newTab;

Why this change?
Attachment #528246 - Flags: ui-review?(limi)
Attachment #528246 - Flags: review?(dolske)
(In reply to comment #154)

> @@ +3636,5 @@
> >  
> >            // We need to select the tab after we've done
> >            // swapBrowsersAndCloseOther, so that the updateCurrentBrowser
> >            // it triggers will correctly update our URL bar.
> > +          this.selectedItem = newTab;
> 
> Why this change?

Boo splinter.

This comment makes more sense when you view it as the original diff:

  -          this.tabbrowser.selectedTab = newTab;
  +          this.selectedItem = newTab;
Blocks: 657097
(In reply to comment #154)

> > +.tabbrowser-tabs[drag=move] > .tabbrowser-tab:not([selected]) {
> > +  -moz-transition: max-width 1ms, -moz-transform 200ms ease-out;
> > +}
> > +

> I really hate how CSS transitions work in this respect... Since CSS only
> applies 1 rule, you can't have independent implementations of things using
> -moz-transition for the same node. [Just as "color: black" and "color:
> white" doesn't give you grey. But this is more akin to deconflicting
> "border: X" and "border: Y" by having separate "border-color: X" and
> "border-style: Y" properties.]

I concur. There are improvements being proposed to the CSSOM that will hopefully solve this problem.

> So, definitely add a comment here, and we should talk with dbaron if there's
> something that can be done with the spec to help in the long run.

Will do.

> >      <content>
> >        <xul:hbox align="end">
> > +        <xul:panel type="drag-image" allowevents="false" hidden="true"
> > +                   class="tab-drag-image" anonid="tab-drag-image"/>
> 
> Dunno if it's specifically from this, but on my Windows debug build, when I
> drag the foreground tab I get an assert in nsMenuPopupFrame.cpp:1147 about
> mPrefSize.width/.height not expected to be zero.
> 
> It's also super slow and janky (like 1Hz update rate, .gifs freeze up too)
> while dragging. Not sure what's up with that, something about my debug
> build, I assume. Hmm, well, my OS X debug build is nice and smooth, though.
> Have you tried this on a Windows debug build?

No, that's odd. I don't have a Windows machine with me (while working remotely), so debugging that may have to wait another week.

> Sigh. I wonder how much time we spend evaluating all the <field>s in this
> file...

I'll make a note in my todo list to benchmark this sometime.

> > +          if (!draggedTab || !draggedTab.parentNode)
> > +            return;
> 
> The .parentNode check is just here to make sure the tab hasn't been closed,
> right?
> 
> Maybe add a <property name="isStillOpen" onget="this.parentNode != null"/>?
> 
> Self-documenting code FTW. :)

Okay. The binding still works even after it's removed from the document? That's interesting.

> > +          if (parent.childNodes.length == 1) {
> 
> parent.childElementCount == 1?

Ooh, right! :)

> > +          // detach if leaving window or at least a tab height down from tabbar
> > +          // Note: we do not have the ability to detect drags over the titlebar
> 
> For future's sake, please expand this comment a tiny bit wrt why the tab
> height bit is done.

Will do.

> > +            // TODO: we should call _blurTab here, but it won't be safe until
> > +            // Panorama is properly integrated and behaves nicely
> 
> Please file a bug for this and note it here.

Bug 657097 filed. Will be noted.

> > +      <method name="_handleTabDrag">
> 
> _handleTabDragover() would be more accurate.

Agreed.

> > +          let draggedTab = event.dataTransfer.mozGetDataAt(TAB_DROP_TYPE, 0);
> > +          if (!draggedTab.parentNode) // tab was closed during drag
> > +            return;
> > +          event.preventDefault();
> > +          event.stopPropagation();
> 
> Is the .stopPropagation really needed here?

IIRC, yes. It prevents the <handler event="dragover"> code from running.

> 1) Part of me wonders if it would be better to avoid the -moz-transforms,
> and instead insert dummy/invisible tab(s), plus controlling their width, in
> order to make the bits slide around. But I don't see anything fundamentally
> _wrong_ with the transform, and that would mean rewriting this from scratch.
> So, fine. :)

I pondered this too, but I found other techniques that I considered (e.g. inserting temporary DOM elements) to have even more risk of creating inconsistent, broken states, especially if a race condition lurked or an unanticipated user action was made.

> 2) While playing with this I noticed a bit of glitchyness on OS X... While
> dragging a tab, sometimes the other tabs would get a :hover hilight applied,
> and then that other tab flickers back to the non-:hover state when you drop
> the tab. I think this can be addressed in a followup if you file it, though.
> :)

I noticed this too early on. I'm not sure if this is my fault or a bug in the :hover calculation. This is possibly related to another glitch that I recently discovered with this patch: it seems to regress bug 465086 randomly. I'd appreciate help in investigating these, since I've made little progress in identifying a cause. I'll file a bug once we at least fix/identify the bug 465086 regression.

> > +            case "dragexit":
> > +              if (aEvent.target == document)
> 
> Why check |.target == document|?

The only time we're sure we want to set the tab to detach state is when the drag is leaving the document. Otherwise, it could be leaving any random child node.

> Sidenote: our documentation sure is vague about the difference/existence of
> dragexit vs dragleave.

AFAIK, dragexit is an event that we've had in XUL for a long time.
https://developer.mozilla.org/en/XUL/Events
Windows Form APIs had a dragleave event, so Microsoft brought that to IE.
When it came to standarding the drag events, the working group folks adopted the IE events, and so we got dragleave.
We fired dragexit events before dragenter events as is sensible.
However, IE fired dragenter events before dragleave events, and the spec adopted their ordering, so we made our code conform too ( bug 527749 ).
Thus, the order of events fired in Gecko is:
dragexit, dragenter, dragleave.

> > +            let right = draggedTab._dragDistX > 0;
> 
> s/right/[dragging|moving]Right/?

Ah yes. This was a case of quickly named variable that never got properly renamed.

> > +            while (next = ltr ^ right ? tabs[--i] : tabs[++i]) {
> 
> Congrats on a rare, legit use of XOR. :)

> > +            return tab._tPos + (ltr ^ right ? 0 : 1);
> 
> ...again. :)

\o/ I relish these operator usage opportunities!

> > +      <method name="_moveTab">
> 
> This method name is a little confusing, given there's already a
> _moveTabTo(). What was it we came up with? slideTab()?

Yeah, _slideTab(). It doesn't slide the tab if the pinned state will change, but once we fix bug 579629, it will.

> > +            let currentTransform = draggedTab.style.MozTransform;
> > +            if (currentTransform && currentTransform != transform) {
> 
> Add comment on this "caching".

It's not caching. If the transform already matches, then no transition will be made, so no transitionend will be called, and the code will fail! Hence the check. Nevertheless, you're absolutely right that it should have a comment! :)

> > +                this.removeEventListener("transitionend", finish, false);
> > +                this.parentNode._finishTabDrag(this);
> > +                this.parentNode.tabbrowser.moveTabTo(this, newIndex);
> 
> Hmm. I suppose this is one of the seemingly few times where an event
> listener's |this| is a useful value.
> 
> Still, I'd posit that it would be worth making this a bit more robust by
> checking event.eventPhase == Event.AT_TARGET, lest this code ever see a
> bubbling transitionend from elsewhere. And maybe s/this/draggedTab/, just to
> avoid the "hey, it's using 
> this|, wonder if it's intentional".

Good idea. I used |this|, because I thought I could avoid pulling things from the closure, but I ended up having to do it anyway.

> > +        if (this.childNodes.length == 1) {
> > +          let windows = Services.wm.getEnumerator("navigator:browser"), n = 0;
> > +          for (; windows.hasMoreElements(); windows.getNext(), n++);
> > +          if (n == 1) // only one tab and one browser window, so nowhere to drop
> > +            return;
> 
> What if there's another window, but it's a popup window? [ala
> nsIBrowserGlue's getMostRecentBrowserWindow()].
> 
> I suppose there's no point in making this code more complex (and potentially
> confusing to the user?) for such an edge case, though.

Is there a quick way to check if a window is a popup window?

> > +        let dragImage = this._tabDragImage;
> > +        dragImage.appendChild(tabPreviews.get(tab));
> > +        dragImage.hidden = false;
> > +        dt.setDragImage(dragImage, -1, -1);
> 
> I'm a little confused here. Why are you doing .setDragImage() here, when the
> panel is also being unhidden?

If I don't call .setDragImage, the default drag image will be the node I'm dragging, so I need to set drag image, in this case, to the <panel>. I keep the <panel> hidden most of the time to avoid it being accidentally rendered or read by screen readers, etc. In other words, setting panel.hidden to false alone does not actually make it appear onscreen nor does it "attach" it to the cursor movements.

> > +      <handler event="drop"><![CDATA[
> > +        this._tabDropIndicator.collapsed = true;
> > +        event.stopPropagation();
> 
> Again with the .stopPropagation()? Oh, I guess this is just moving code. Hmm.

Yeah, I just moved code. I simply decided not to risk touching it.

> @@ +3610,5 @@
> > +        if (draggedTab) {
> > +          let parent = draggedTab.parentNode;
> > +          if (parent == this) {
> > +            // It is better to handle tab moving on drop when possible,
> > +            // since there is a lag between the drop and dragend events.
> 
> Did you investigate and/or file a bug on the apparent delay?

I investigated it on OS X, and it seems that the delay does not exist; I'll confirm on Windows and Linux asap. (Well, there is a delay, but it's ~1ms (tested with both a simple HTML testcase and tabbrowser.xml), so I'm not worried about it.) I guess it was a bug with an old version of my patch or something.

Consequently, I removed the call to _slideTab() from the drop handler. Since now the only caller of _slideTab() is the "dragend" handler, should I leave it as a separate function or inline it?
Blocks: animations
When it will be added to the nightly?
when it will be ready
We never see this in firefox 7?
Frank Yan, could you please push this feature to UX branch?
Attached patch patch v3 (obsolete) — Splinter Review
Unbitrotted and addressed review comments. Not ready for review. 1 remaining issue :(
Ready for help on Linux when you're available, Neil. Thanks in advance :)
Attachment #528246 - Attachment is obsolete: true
(In reply to comment #162)
> Created attachment 540322 [details]
> Last tab drag drop changes nav bar height

Disable Stratiform, if this still happen, disable all the add-ons and plug-in, use the default theme
(In reply to comment #162)
> Created attachment 540322 [details]
> Last tab drag drop changes nav bar height

Disable Stratiform, if this still happen, disable all the add-ons and plug-in, use the default theme without any personas.
Happens again , but with less magnitude , i.e. the height does reduce , but by less px
Comment on attachment 528247 [details] [diff] [review]
addendum to disable detach panel on Linux for now

(Clearing old review request for linux workaround. Should be fine if this ends up landing before but 533460 is fixed, though.)
Attachment #528247 - Flags: review?(dolske)
This is somewhat a reference back to comment #19, but I've just noticed (in the UX nightly) that attempting to duplicate the tabs (by pressing ctrl and dragging) instead moves the tab into a new window. Expected behavior, of course, would be to create a duplicate tab
Attached patch patch v4 (obsolete) — Splinter Review
This fixes the regression of bug 465086.
The cause was the destruction and recontruction of the tabbrowser-tab binding whenever a tab moved around in the DOM. The solution is to make _fullyOpen a regular JS expando property, not an XBL field.

This also includes a partial workaround for bug 666864, which causes the incorrect application of :hover styling to a background tab during and after a drag operation.
Attachment #528247 - Attachment is obsolete: true
Attachment #539762 - Attachment is obsolete: true
Attachment #541621 - Flags: review?(dolske)
Attachment #541621 - Flags: ui-review?(limi)
Depends on: 667177
No longer depends on: 667177
Depends on: 666864
Attached patch patch v5 (obsolete) — Splinter Review
Fixes issue illustrated by https://bug455694.bugzilla.mozilla.org/attachment.cgi?id=540322

Also fixes a typo and a few edge cases.
Attachment #540322 - Attachment is obsolete: true
Attachment #541621 - Attachment is obsolete: true
Attachment #542302 - Flags: ui-review?(limi)
Attachment #542302 - Flags: review?(dolske)
Attachment #541621 - Flags: ui-review?(limi)
Attachment #541621 - Flags: review?(dolske)
Comment on attachment 542302 [details] [diff] [review]
patch v5

We've been running this on the UX branch for the past weeks, and it has worked great so far. Let's land it!
Attachment #542302 - Flags: ui-review?(limi) → ui-review+
YAYYY !!
Lets do it
BTW I know its off topic, But Alexander Limi , why was the new tab prototype v1 removed from ux branch ?
May be for v2 ?
(In reply to comment #170)
> Comment on attachment 542302 [details] [diff] [review] [review]
> patch v5
> 
> We've been running this on the UX branch for the past weeks, and it has
> worked great so far. Let's land it!

But it's still laggy when you move tabs fast enough on Windows (7, and I don't know anything about other platforms). Chrome and Opera have no tab animation lag...
(In reply to comment #173)
> But it's still laggy when you move tabs fast enough on Windows (7, and I
> don't know anything about other platforms). Chrome and Opera have no tab
> animation lag...

Confirmed!
Please do not landed in this form. Get the disappearance of lags, then add.
Yes , the tab dragging animation is lagging a lot, slow and feels heavy. Chrome's tab dragging animation is smooth and feels so light.
Not sure which patch was applied on Firefox UX branch but it looks like that this patch broke function "Duplicate tab" (press CTRL and move tab). It works fine on latest trunk.
Comment on attachment 542302 [details] [diff] [review]
patch v5

Random drive-by comments:

>--- a/browser/base/content/browser.css
>+++ b/browser/base/content/browser.css
>@@ -57,16 +57,40 @@ tabbrowser {
>   -moz-transition: opacity 250ms;
> }
> 
> .tabbrowser-tabs[positionpinnedtabs] > .tabbrowser-tab[pinned] {
>   position: fixed !important;
>   display: block; /* position:fixed already does this (bug 579776), but let's be explicit */
> }
> 
>+.tabbrowser-tab[selected] {
>+  z-index: 2;
>+}

It's not clear what this is supposed to do, needs a comment.

>       <method name="addTab">
>         <parameter name="aURI"/>
>-        <parameter name="aReferrerURI"/>
>+        <parameter name="aReferrerURI"/>up
>         <parameter name="aCharset"/>
>         <parameter name="aPostData"/>
>         <parameter name="aOwner"/>
>         <parameter name="aAllowThirdPartyFixup"/>

...

>       <handler event="underflow" phase="capturing"><![CDATA[
>-        if (event.detail == 0)
>+        if (event.target != this || event.detail == 0)
>           return; // Ignore vertical events

This looks wrong to me. I think the target is supposed to be the <scrollbox>, not the <arrowscrollbox>. If this works as is, then that's probably due to xbl being involved. In that case, check event.originalTarget.

>+          if (event.type == "dragexit" || event.screenY > bo.screenY + 2 * bo.height) {
>+            if (parent.getAttribute("drag") != "detach") {
>+              parent.setAttribute("drag", "detach");
>+              parent._clearTransforms();
>+              parent._tabDropIndicator.collapsed = true;
>+              delete draggedTab._dropIndex;
>+              if (parent.tabbrowser.visibleTabs.length == 1)
>+                draggedTab.style.maxWidth = "";
>+            }
>+            // TODO: we should call _blurTab here, but it won't be safe until
>+            // Panorama is properly integrated and behaves nicely

Unclear what this means. Why should _blurTab be called and what are the consequences of not doing it? What's the problem with panorama?

>+      <method name="_clearTransforms">

Needs a more distinctive name, _dragClearTransforms if nothing else; doesn't need to be super-nice since it's private.
(In reply to comment #178)

> >+.tabbrowser-tab[selected] {
> >+  z-index: 2;
> >+}
> 
> It's not clear what this is supposed to do, needs a comment.

Will add the comment: /* keep selected tab on top during drags */

> >       <handler event="underflow" phase="capturing"><![CDATA[
> >-        if (event.detail == 0)
> >+        if (event.target != this || event.detail == 0)
> >           return; // Ignore vertical events
> 
> This looks wrong to me. I think the target is supposed to be the
> <scrollbox>, not the <arrowscrollbox>. If this works as is, then that's
> probably due to xbl being involved. In that case, check event.originalTarget.

It works as is, but I'll change it to check event.originalTarget.

> >+            // TODO: we should call _blurTab here, but it won't be safe until
> >+            // Panorama is properly integrated and behaves nicely
> 
> Unclear what this means. Why should _blurTab be called and what are the
> consequences of not doing it? What's the problem with panorama?

Ideally, the UI should preview to the user what will happen when the user releases the pointing device. That means, when the user drags the tab beyond the detachment threshold, we should call some form of _blurTab immediately, so the user clearly sees that the tab will be detached and what the UI will look like upon release. However, in the case of having only one visible tab, if there could be other tab groups, we would immediately switch to tab group view, but then it becomes more difficult for the user to undo that action by dragging the tab back to the tab strip, because it's no longer visible! In addition, tab group view currently doesn't accept TAB_DROP_TYPE drops, and it's strange anyway to undo an action by dragging the tab to another location. Making tab groups global would mitigate this problem, since then we wouldn't have to switch to tab group view.

> >+      <method name="_clearTransforms">
> 
> Needs a more distinctive name, _dragClearTransforms if nothing else; doesn't
> need to be super-nice since it's private.

Will rename it to _dragClearTransforms.

Thanks for the feedback. :)
> > >+      <method name="_clearTransforms">
> > 
> > Needs a more distinctive name, _dragClearTransforms if nothing else; doesn't
> > need to be super-nice since it's private.
> 
> Will rename it to _dragClearTransforms.

If you swap the two verb-sounding words, you end up with the much less confusing _clearDragTransforms.
Attached patch patch v6 [ui-r=limi] (obsolete) — Splinter Review
Addressed Dão's comments.
Fixed another bug 465086 regression. In the process of that, fixed bug 652735. (It's more complex not to fix both simultaneously.)

(In reply to comment #180)
> If you swap the two verb-sounding words, you end up with the much less
> confusing _clearDragTransforms.

Fixed. Thanks!
Attachment #542302 - Attachment is obsolete: true
Attachment #543287 - Flags: review?(dolske)
Attachment #542302 - Flags: review?(dolske)
Attached patch the real patch v6 [ui-r=limi] (obsolete) — Splinter Review
Removed accidental keystrokes on random line of file.
Attachment #543287 - Attachment is obsolete: true
Attachment #543288 - Flags: review?(dolske)
Attachment #543287 - Flags: review?(dolske)
Actually removed accidental keystrokes on random line of file.
My text editor and I are not getting along today. Sorry. :(
Attachment #543288 - Attachment is obsolete: true
Attachment #543289 - Flags: review?(dolske)
Attachment #543288 - Flags: review?(dolske)
Attached patch patch v7 [ui-r=limi] (obsolete) — Splinter Review
The drag & drop API did not provide the performance we needed, and it caused glitches like bug 666864, so I switched to using plain mouse events, which enabled an implementation that is noticeably more responsive, especially in the common case of rearranging tabs.

Using this approach removes the dependencies on bug 533460 and bug 666864. IMHO, it's a cleaner approach, because it only makes sense for Firefox tab strips to accept the drops of Firefox tabs, so no other elements should react to a Firefox tab being dragged over it. For example, using the drag & drop API, the OS X dock popped up when dragging to the bottom of the screen, even though it couldn't accept the drop of a tab. The new patch doesn't have this problem. 

It turns out that attaching a mousemove event listener to the document in a dragstart listener also captures events outside of the window, which is what we want. There is no need for element.setCapture() because we actually want the original targets for each event. I used dragstart instead of mousedown to initialize the drag, because it provides a convenient cursor jitter threshold before it is triggered.
Attachment #543289 - Attachment is obsolete: true
Attachment #543394 - Flags: review?(dolske)
Attachment #543289 - Flags: review?(dolske)
No longer depends on: 533460, 666864
(In reply to comment #179)
> (In reply to comment #178)
> 
> > >+.tabbrowser-tab[selected] {
> > >+  z-index: 2;
> > >+}
> > 
> > It's not clear what this is supposed to do, needs a comment.
> 
> Will add the comment: /* keep selected tab on top during drags */

Why is this applied regardless of the tab being dragged?
@Frank , when will it land in ux branch? , need to try it out , sounds awesome by the way :)
(In reply to comment #185)
> (In reply to comment #179)
> > (In reply to comment #178)
> > 
> > > >+.tabbrowser-tab[selected] {
> > > >+  z-index: 2;
> > > >+}
> > > 
> > > It's not clear what this is supposed to do, needs a comment.
> > 
> > Will add the comment: /* keep selected tab on top during drags */
> 
> Why is this applied regardless of the tab being dragged?

Because it doesn't really make a difference either way when the tab is not being dragged. Do you have a good reason for making the selector more complex and limiting it to |.tabbrowser-tabs[drag] > .tabbrowser-tab[selected]| ?

(In reply to comment #186)
> @Frank , when will it land in ux branch? , need to try it out , sounds
> awesome by the way :)

Probably tomorrow. Instead of writing another comment unrelated to development that causes an email to be sent out to 155 people, please check the logs yourself: https://hg.mozilla.org/projects/ux/log?rev=frank
(In reply to comment #187)
> (In reply to comment #185)
> > (In reply to comment #179)
> > > (In reply to comment #178)
> > > 
> > > > >+.tabbrowser-tab[selected] {
> > > > >+  z-index: 2;
> > > > >+}
> > > > 
> > > > It's not clear what this is supposed to do, needs a comment.
> > > 
> > > Will add the comment: /* keep selected tab on top during drags */
> > 
> > Why is this applied regardless of the tab being dragged?
> 
> Because it doesn't really make a difference either way when the tab is not
> being dragged. Do you have a good reason for making the selector more
> complex and limiting it to |.tabbrowser-tabs[drag] >
> .tabbrowser-tab[selected]| ?

It's just confusing. I don't see any complexity in the full selector.
Attached patch patch v8 [ui-r=limi] (obsolete) — Splinter Review
Addressed Dão's comment.
Fixed an if condition that caused tab sizing to unlock when it shouldn't have.
Removed an unnecessary if condition.
Attachment #543394 - Attachment is obsolete: true
Attachment #543421 - Flags: review?(dolske)
Attachment #543394 - Flags: review?(dolske)
Comment on attachment 543421 [details] [diff] [review]
patch v8 [ui-r=limi]

Review of attachment 543421 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't do a nitty-gritty review of the math stuff yet, just from a general view it's looking ok. Alas, r- for the issues noted in tabbrowser.xml.

::: browser/base/content/tabbrowser.xml
@@ +1485,5 @@
>              if (!this._beginRemoveTab(aTab, false, null, true))
>                return;
>  
> +            if (aTab.selected && this.tabContainer.hasAttribute("drag"))
> +              this._endTabDrag(aTab);

Should be this.tabContainer._endTabDrag?

[This was the bug we found... Press Command-W while dragging a tab, drop it at the end, weird stuff happens.]

@@ +2782,5 @@
>        <field name="_tabDropIndicator">
>          document.getAnonymousElementByAttribute(this, "anonid", "tab-drop-indicator");
>        </field>
>  
> +      <method name="_positionDropIndicator">

Sigh, XBL. It would be nice to keep this code unmoved (for reviewer sanity and blame history), but <handler> vs <method>. :(

@@ +2787,5 @@
> +        <parameter name="event"/>
> +        <parameter name="scrollOnly"/>
> +        <body><![CDATA[
> +          if (event.dataTransfer)
> +            var effects = this._setEffectAllowedForDataTransfer(event);

Init effects to "" or in an else here?

@@ +2920,5 @@
> +              event.screenX >= bo.screenX + bo.width ||
> +              event.screenY < bo.screenY - (detached ? 0 : 1) * bo.height ||
> +              event.screenY >= bo.screenY + (detached ? 1 : 2) * bo.height) {
> +            state = true;
> +            if (!this._isEventInsideWindow(event, window)) {

From Friday review: |window| is an unused arg here.  Consider caching result in a local var earlier, to avoid calling twice?

@@ +2924,5 @@
> +            if (!this._isEventInsideWindow(event, window)) {
> +              // cycle through windows from front to back:
> +              // if the cursor is inside the tab strip of a window,
> +              // indicate where the tab would be dropped
> +              let windows = Services.wm.getZOrderDOMWindowEnumerator("navigator:browser", true);

So... Bug 462222 talks about this being broken on Mac and Linux (and OS/2). :( It seems those implementations just use an oldest-to-newest window enumerator to fake it, instead of a real top-to-bottom mechanism. This sorta worked in the past because it was typically just used for get the topmost-ish window to open a tab in, but it looks like this could be pretty bad for your usecase.

nsIWindowMediator has some other z-order checking methods, but they're all marked [noscript] (and I'm not sure if they're also broken!). You may need to do some widget hacking to figure this out. I'd sorta expect it shouldn't be that hard, but just hasn't been important until now.

Also, shouldn't something be checking to see if there's some other window obscuring the background Firefox tabstrip?

@@ +3717,5 @@
> +
> +        document.addEventListener("mousemove", this);
> +        document.addEventListener("mouseup", this);
> +        this.mTabstrip.addEventListener("scroll", this);
> +        tab.addEventListener("TabAttrModified", this);

It would be good to add a comment (and maybe test!) explaining when this case can happen.

@@ +3736,5 @@
> +        // valid urls don't contain spaces ' '; if we have a space it isn't a valid url.
> +        // Also disallow dropping javascript: or data: urls--bail out
> +        if (!url || !url.length || url.indexOf(" ", 0) != -1 ||
> +            /^\s*(javascript|data):/.test(url))
> +          return;

Hmm, this should probably be a whitelist to only allow certain URLs. New bug, natch.
Attachment #543421 - Flags: review?(dolske) → review-
I'm getting two issues : 
1.The detached tab is leaving behind a 4px tab on the tab strip (always). 
2.The tab panel of the detached tab is randomly sometimes white/blank
Attached patch patch v9 [ui-r=limi] (obsolete) — Splinter Review
(In reply to comment #190)
> > +      <method name="_positionDropIndicator">
> 
> Sigh, XBL. It would be nice to keep this code unmoved (for reviewer sanity
> and blame history), but <handler> vs <method>. :(

Indeed. I call it in two places, so I had to move it.

> > +              let windows = Services.wm.getZOrderDOMWindowEnumerator("navigator:browser", true);
> 
> So... Bug 462222 talks about this being broken on Mac and Linux (and OS/2).
> [...]
> Also, shouldn't something be checking to see if there's some other window
> obscuring the background Firefox tabstrip?

Yeah, I'll have to look into these issues. :(

> > @@ +3736,5 @@
> > // valid urls don't contain spaces ' '; if we have a space it isn't a valid url.
> > // Also disallow dropping javascript: or data: urls--bail out
> > if (!url || !url.length || url.indexOf(" ", 0) != -1 ||
> >     /^\s*(javascript|data):/.test(url))
> >   return;
> 
> Hmm, this should probably be a whitelist to only allow certain URLs. New
> bug, natch.

That was just a de-indentation of old code. I filed bug 669046 to fix it.

The other review comments have been addressed.
I'm not yet requesting review, because the z-order issue still needs to be resolved. I have ideas for it though.
Attachment #543421 - Attachment is obsolete: true
(In reply to comment #191)
> I'm getting two issues : 
> 1.The detached tab is leaving behind a 4px tab on the tab strip (always). 
> 2.The tab panel of the detached tab is randomly sometimes white/blank

3. The taskbar Preview Has stopped showing tab thumbnail images (only loading icon shown)and this is happening since the launch of tab animation. I've kept cache frequency to 0 for realtime.
Thank you so much for deliverance from the lags when dragging tabs.
There is no way to drag the inactive tab
Sorry to derail the conversation, but people seem to be able to test this bugfix, and I cannot for the life of me figure out how. I'm new to Bugzilla, and I want to be able to test the bugs. Thanks.
(In reply to comment #196)
> Sorry to derail the conversation, but people seem to be able to test this
> bugfix, and I cannot for the life of me figure out how. I'm new to Bugzilla,
> and I want to be able to test the bugs. Thanks.

Latest UX Branch is available here (yesterday Tab Animation was finally smooth):
https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-ux/
(In reply to comment #198)

> Latest UX Branch is available here (yesterday Tab Animation was finally
> smooth):
> https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-ux/

Well, I tried it. Apart from the (already mentioned) inability to drag to bookmarks, I can't alt+tab between firefox windows, which I think is an even more annoying regression. I try to drag to bookmarks, which creates a new windows instead, but I can't even move it back to the original window anymore, without resizing and arranging the widows properly. Also in this case the old drop arrow appears instead of the new animation...
(In reply to comment #199)
> bookmarks, I can't alt+tab between firefox windows, which I think is an even
> more annoying regression. I try to drag to bookmarks, which creates a new
> windows instead, but I can't even move it back to the original window
> anymore, without resizing and arranging the widows properly. Also in this
> case the old drop arrow appears instead of the new animation...

This sounds pretty bad. Alt+Tab is indeed an important tool when dragging between windows, especially maximized windows.
Are you going to make an option not to create new windows via detaching at all? To my mind to drag to bookmarks is MUCH MORE important thing than to create a new window (by the way, I don't use it at all). Thus I would like to turn this off and simply to have nice animation when I reorder tabs.
(In reply to comment #201)
> drag to bookmarks is MUCH MORE important

FWIW, you can also bookmark a tab by dragging its identity box to bookmarks toolbar.
I know there are several ways to add a bookmark e.g. using a button on the address bar or, as it's described a few lines above, dragging its identity box or maybe something else but the way I used (dragging a tab) is the most convenient for me. Also when I browse quickly the mouse drag movement can drop and I may by mistake create really annoying new windows. Oh, I know that some people may like this feature (I don't ask you to turn this on by default), but I don't. I like Firefox (maybe like many users) because it have a LOT of options allowing to configure its behavior which include tab behavior too, and I think such option is really necessary and not difficult to do. Excuse me if you don't think so.
I think it's out of the scope of this bug since this feature is already present in Firefox 4.
(In reply to comment #203)
> I know there are several ways to add a bookmark e.g. using a button on the
> address bar or, as it's described a few lines above, dragging its identity
> box or maybe something else but the way I used (dragging a tab) is the most
> convenient for me. Also when I browse quickly the mouse drag movement can
> drop and I may by mistake create really annoying new windows. Oh, I know
> that some people may like this feature (I don't ask you to turn this on by
> default), but I don't. I like Firefox (maybe like many users) because it
> have a LOT of options allowing to configure its behavior which include tab
> behavior too, and I think such option is really necessary and not difficult
> to do. Excuse me if you don't think so.

All this is irrelevant to this bug , please do not discuss about bookmarking here. Also , accidentally making new windows is no bug , its just like accidentally closing firefox etc
I'm very much enjoying to look & feel of the new animations! However, I've found a problem regarding window sizes on Windows Vista:

1. In a maximized window, open a new tab.
2. Drag the tab down into the content window and release to open it into a new browser window.
3. The new window is maximized; restore it.
4. Drag the tab back into the maximized window.
5. Restore the maximized window.

You will now see that the window has been resized to the screen resolution. It makes more sense to me that the window's restore size is left unchanged.
Blocks: 597989
Attached patch patch v10 [ui-r=limi] (obsolete) — Splinter Review
- Addressed review comments.
- Addressed obscured window problem by fixing bug 597989.
- Removed bogus tab stub on Windows.
- Restored the ability to cancel the drag by pressing the esc key, even while the drag image is not visible or another window is focused.
- Mitigated problems with the z-order window enumerator.
Attachment #543660 - Attachment is obsolete: true
Attachment #544950 - Flags: review?(dolske)
Comment on attachment 544950 [details] [diff] [review]
patch v10 [ui-r=limi]

Oops:
> +.tabbrowser-tabs[drag=detach] > .tabbrowser-tab[selected],
> +.tabbrowser-tabs[drag=teleport] > .tabbrowser-tab[selected] {

The above two lines should be replaced by the following:
> +.tabbrowser-tabs[drag=detach] > .tabbrowser-tab[selected] {

("teleport" was a placeholder value used in an approach that I abandoned.)
Is Alt+Tab for moving tabs between windows fixed?
Attached patch patch v11 [ui-r=limi] (obsolete) — Splinter Review
(In reply to comment #209)
> Is Alt+Tab for moving tabs between windows fixed?

Now, yes, at the cost of being able to cancel the drag with the esc key, because setting noautohide to true also causes that shortcut to be disabled.
Attachment #544950 - Attachment is obsolete: true
Attachment #544974 - Flags: review?(dolske)
Attachment #544950 - Flags: review?(dolske)
Can we finally merge this into trunk?
Attached patch patch v12 [ui-r=limi] (obsolete) — Splinter Review
It turns out that <panel level="top"/> doesn't work on Linux, so I had to put in an #ifdef'd workaround for that.

The primary change from v10 to v11 is the vast improvement in theming. The drag image now approximates the shape of a window or tab correponding to the current drop target. So far, it looks best on Windows Vista/7 for which borderless, transparent panels are easiest to create. Further cosmetic improvements will be left to a followup.

I'll upload a screencast of this patch in action on Windows 7 shortly.
Attachment #544974 - Attachment is obsolete: true
Attachment #545128 - Flags: review?(dolske)
Attachment #544974 - Flags: review?(dolske)
If it lands in a few days on m-c , will it make it through for firefox 7 ?
Attached video Screencast of patch v12 on Windows 7 (obsolete) —
(In reply to comment #213)
> If it lands in a few days on m-c , will it make it through for firefox 7 ?

No, Firefox 7 moved to Aurora on 2011-07-05, mozilla-central work is now for Firefox 8.

See:
https://wiki.mozilla.org/Releases
https://wiki.mozilla.org/RapidRelease/Calendar
in the screencast of v12 when we detach the tab from firefox windows the cursor comes to the center of the miniature...it's not better to cursor stay in the top of miniature?
Attached patch patch v13 [ui-r=limi] (obsolete) — Splinter Review
Attachment #545128 - Attachment is obsolete: true
Attachment #545561 - Flags: review?(dolske)
Attachment #545128 - Flags: review?(dolske)
(In reply to comment #214)
> Created attachment 545132 [details]
> Screencast of patch v12 on Windows 7

Very nice!

One note though, when we go from window to tab (0:11 in the video), is it possible to make it a "real" tab, that moves other tabs away. It looks like a flying label now.
The real Tab would be very nice , instead of flying label.

Two issues I am having on Windows 7 (with the latest ux nightly build)

1. The new tab button disappears as soon as I detach a tab .
2. Not able to drag the detached tab panel to the task bar. This scenario occurs when the detached tab is from a maximized window and I want it to drag and drop it to another maximized window. Thus I have to hover the mouse to the corresponding taskbar button, but I'm unable to do that as the detached tab is not entering the taskbar.
(In reply to comment #220)
> Created attachment 545668 [details]
> The thumbnail while detaching goes transparent on light backgrounds

It would be appreciated if you could convert videos to WebM in future. :) Miro make a good converter that you can use.

http://www.mirovideoconverter.com/
apologizes , i didnt have any nice converter atm , i was thinking to convert it to GIF
(In reply to comment #152)
> I found something : You could call it a bug or a missing feature.
> 
> When we have 2 windows of firefox open. If we detach one tab from one of the
> window(say window 1) and drag it to the tab strip of the other window(say
> windows 2), the tab from the tab strip of window 1 is removed , but no tab
> is added to the tab strip of window 2. Just the arrow is shown. 
> 
> What is required would be something like this : 
> we detach a tab from window 1 , the tab from the tab strip of Window 1
> immediately disappears. And when we add that tab to Window 2, its tab should
> be added to Window 2's tab strip along with the tab rearrangement animation
> in action.
> 
> I hope my idea is clear to all and I wish to see this feature in this
> animation.

When will this feature be implemented? It think it's cool and Chrome already has it. I wish to see this feature too.
It would be great if the window disappears when the last tab is detached like in Chrome. Otherwise very good work, the animations are smooth :)
Can anyone reproduce the bug that the new tab button disappears on tab detach ?
Also that the detached panel cannot be moved on to taskbar in windows 7
(In reply to comment #225)
> Can anyone reproduce the bug that the new tab button disappears on tab
> detach ?
> Also that the detached panel cannot be moved on to taskbar in windows 7

Yes, I have the same bug in Windows 7. I think everyone can reproduce this.
(In reply to comment #226)
> (In reply to comment #225)
> > Can anyone reproduce the bug that the new tab button disappears on tab
> > detach ?
> > Also that the detached panel cannot be moved on to taskbar in windows 7
> 
> Yes, I have the same bug in Windows 7. I think everyone can reproduce this.

I can reproduce it too.
I can reproduce it on the Mac build as well.

The New Tab button disappears on tab detach and I can't drag a tab over the Dock.

Hovering the Dock while dragging a tab is as important as the Windows 7 Taskbar, since it's one of the ways to trigger Application-specific Exposé and have it showcase all the application's windows miniatures on desktop, to pick the correct one to attach the tab to.
(In reply to comment #218)
> One note though, when we go from window to tab (0:11 in the video), is it
> possible to make it a "real" tab, that moves other tabs away. It looks like
> a flying label now.

We already have plans for many more improvements including this after landing this first.

> 1. The new tab button disappears as soon as I detach a tab .

Intentional behavior to mitigate issue noted in comment 136 until followup work is completed.

> 2. Not able to drag the detached tab panel to the task bar. This scenario
> occurs when the detached tab is from a maximized window and I want it to
> drag and drop it to another maximized window. Thus I have to hover the mouse
> to the corresponding taskbar button, but I'm unable to do that as the
> detached tab is not entering the taskbar.

Alt+tab. (Cmd+` on OS X.)

(In reply to comment #220)
> Created attachment 545668 [details]
> The thumbnail while detaching goes transparent on light backgrounds

Thanks for the catch. I'll try to fix this. Please convert these to WebM using the awesome, free Miro Video Converter.
(In reply to comment #229)
> Alt+tab. (Cmd+` on OS X.)

Yes, as a temporary workaround it's okay (or use the keyboard shortcut set for Exposé, which is what I'm doing right now), but it'd be nice as a future improvement if one could use the Dock/Taskbar without having to reach for the keyboard.
On my Vista installation, dragging the tab preview tab initially works excellently, until pressing Alt+Tab to switch to a different Fx window. Occasionally, the dragged window then remains stationary until I move the cursor outside of the window. I find this very hard to reproduce, but it seems that when it happens, both windows are maximized.

I've also noticed that if I move my mouse very abruptly, my cursor will exit the window and the dragged tab will not follow it until I mouse-over it again.
> 2. Not able to drag the detached tab panel to the task bar. This scenario
> occurs when the detached tab is from a maximized window and I want it to
> drag and drop it to another maximized window. Thus I have to hover the mouse
> to the corresponding taskbar button, but I'm unable to do that as the
> detached tab is not entering the taskbar.

It's also connected with inability to Show Desktop, even if mouse is pointing at the "Show Desktop" button.
(In reply to comment #220)
> Created attachment 545668 [details]
> The thumbnail while detaching goes transparent on light backgrounds

I can't reproduce this. Are you running any theme or appearance-related extensions? Could you temporarily removing everything (including any @namespace lines) from your profile's userChrome.css file and simply putting the following line in the file:
.tab-drag-thumbnail { background-color: white; background-clip: content-box; }
(In reply to comment #233)
> (In reply to comment #220)
> > Created attachment 545668 [details]
> > The thumbnail while detaching goes transparent on light backgrounds
> 
> I can't reproduce this. Are you running any theme or appearance-related
> extensions?

As a Windows developer I recognize this phenomenon: it is caused by Aero Glass, which uses black as glassy transparency.  There's a resolution to this, but I only have Java native interface code, which I imagine isn't very useful.

You could best reproduce it by dragging a tab with a mostly black preview.
(In reply to comment #234)
> As a Windows developer I recognize this phenomenon: it is caused by Aero
> Glass, which uses black as glassy transparency. [...]
> 
> You could best reproduce it by dragging a tab with a mostly black preview.

I'm aware of the phenomenon. We ran into that problem a lot in our early attempts to make Firefox 4 use Aero Glass. I was unable to reproduce it by dragging tabs with thumbnails of any color.

If you can reproduce this bug, could you navigate to about:support and paste the text in the Graphics section in a comment here?
(In reply to comment #235)
> If you can reproduce this bug, could you navigate to about:support and paste
> the text in the Graphics section in a comment here?

  Graphics

        Adapter Description
        ATI Radeon HD 4800 Series

        Vendor ID
        1002

        Device ID
        9442

        Adapter RAM
        512

        Adapter Drivers
        aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64

        Driver Version
        8.861.0.0

        Driver Date
        5-24-2011

        Direct2D Enabled
        true

        DirectWrite Enabled
        true (6.1.7600.16699)

        ClearType Parameters
        Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 300

        WebGL Renderer
        Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686)

        GPU Accelerated Windows
        1/1 Direct3D 10
I can too

Graphics:

Adapter Description 
ATI Mobility Radeon HD 5650

Vendor ID 
1002

Device ID 
68c1

Adapter RAM 
1024

Adapter Drivers 
atiu9p64.dll atiuxp64 atiuxp64 atiu9pag atiuxpag atiuxpag atiumdva atiumd6a atitmm64

Driver Version 
8.771.1.0

Driver Date 
9-9-2010

Direct2D Enabled 
true

DirectWrite Enabled 
true (6.1.7601.17563)

ClearType Parameters 
ClearType parameters not found

WebGL Renderer 
Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686)

GPU Accelerated Windows 
1/1 Direct3D 10
Graphics:

Adapter Description
NVIDIA GeForce 8600 GTS

Vendor ID
10de

Device ID
0400

Adapter RAM
256

Adapter Drivers
nvd3dum nvwgf2um,nvwgf2um

Driver Version
8.17.12.6724

Driver Date
2-23-2011

Direct2D Enabled
true

DirectWrite Enabled
true (7.0.6002.18409)

ClearType Parameters
ClearType parameters not found

WebGL Renderer
Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686)

GPU Accelerated Windows
1/1 Direct3D 10
I can reproduce the thumbnail issue. Also the animations are a little bit sluggish.

Adapter Description: NVIDIA GeForce 8800 GT
Vendor ID: 10de
Device ID: 0611
Adapter RAM: 512
Adapter Drivers: nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Driver Version: 8.17.12.7533
Driver Date: 5-20-2011
Direct2D Enabled: true
DirectWrite Enabled: true (6.1.7601.17563)
ClearType Parameters: ClearType parameters not found
WebGL Renderer: Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.686)
GPU Accelerated Windows: 1/1 Direct3D 10
I can't reproduce this bug. Looks like the difference here is that Direct2D and DirectWrite are disabled on my system. I don't see how DirectWrite can affect this, so perhaps the problem lies in Direct2D?

  Graphics

        Adapter Description
        ATI Mobility Radeon X1300

        Vendor ID
        1002

        Device ID
        714a

        Adapter RAM
        Unknown

        Adapter Drivers
        atiumdag atiumdva atitmmxx

        Driver Version
        8.561.0.0

        Driver Date
        12-1-2008

        Direct2D Enabled
        Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer.

        DirectWrite Enabled
        false (6.1.7600.16763)

        ClearType Parameters
        ClearType parameters not found

        WebGL Renderer
        Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer.

        GPU Accelerated Windows
        0/1. Blocked for your graphics driver version. Try updating your graphics driver to version 10.6 or newer.
(In reply to comment #195)
> There is no way to drag the inactive tab

This still seems to be an issue (on Mac at least).

Specifically:
- If the current tab is the leftmost one and you drag any other tab to move it, nothing will happen
- If the current tab isn't the leftmost one and you drag any other tab to move it, the current tab will jump to the leftmost position

http://alimonda.com/firefoxtabdrag.webm
(In reply to comment #241)
> (In reply to comment #195)
> > There is no way to drag the inactive tab
> 
> This still seems to be an issue (on Mac at least).
> 
> Specifically:
> - If the current tab is the leftmost one and you drag any other tab to move
> it, nothing will happen
> - If the current tab isn't the leftmost one and you drag any other tab to
> move it, the current tab will jump to the leftmost position
> 
> http://alimonda.com/firefoxtabdrag.webm

In windows too
(In reply to comment #242)
> In windows too

After checking other people's experiences on the mozillazine UX branch thread ([1]), it ended up being an extension's fault.  It's likely to be the same for you, if you use Tab Utilities or a similar addon.  WFM on a clean profile.

[1]: http://forums.mozillazine.org/viewtopic.php?p=11027729#p11027729
Attached patch patch v14 [ui-r=limi] (obsolete) — Splinter Review
Fixed the transparent panel glitch caused by bizarre Aero Glass interactions.
Thanks for listening your graphics driver information. We don't need any more.

The drag preview now uses a live thumbnail!
The drag preview on OS X looks more like a window.
The new tab button is hidden only when there are no remaining visible tabs.
Tabs animate to fill the space of a tab being detached.
Attachment #463754 - Attachment is obsolete: true
Attachment #477449 - Attachment is obsolete: true
Attachment #545132 - Attachment is obsolete: true
Attachment #545561 - Attachment is obsolete: true
Attachment #545668 - Attachment is obsolete: true
Attachment #546359 - Flags: review?(dolske)
Attachment #545561 - Flags: review?(dolske)
Blocks: 672085
Comment on attachment 546359 [details] [diff] [review]
patch v14 [ui-r=limi]

>--- a/browser/themes/winstripe/browser/browser-aero.css
>+++ b/browser/themes/winstripe/browser/browser-aero.css

>+%define panelBorder rgba(10%,10%,10%,.7) rgba(255,255,255,.7)
>+%define panelGlass rgba(255,255,255,.3) rgba(255,255,255,.3) rgba(255,255,255,.3) rgba(255,255,255,.3)
>+%define previewBorder rgba(20%,20%,20%,.7)

>+  .tab-drag-panel {
>+    -moz-appearance: -moz-win-borderless-glass;
>+    border-radius: 7px;
>+  }
>+  .tab-drag-label {
>+    padding: 4px;
>+    background-color: -moz-dialog;
>+    border: 1px solid DimGray;
>+    border-radius: 3px;
>+  }
>+  .tab-drag-panel:not([target]) > .tab-drag-label {
>+    display: none;
>+  }
>+  .tab-drag-preview {
>+    display: block;
>+    margin: 5px;
>+    box-shadow: 0 0 5px rgba(10%,10%,10%,.7);
>+    border-width: 15px 7px 7px;
>+    border-style: solid;
>+    -moz-border-top-colors: @panelBorder@ @panelGlass@ @panelGlass@ @panelGlass@ @previewBorder@;
>+    -moz-border-right-colors: @panelBorder@ @panelGlass@ @previewBorder@;
>+    -moz-border-bottom-colors: @panelBorder@ @panelGlass@ @previewBorder@;
>+    -moz-border-left-colors: @panelBorder@ @panelGlass@ @previewBorder@;
>+    border-radius: 7px;
>+  }
>+
>+  .tab-drag-preview::before {
>+    content: "";
>+    display: block;
>+    margin: -14px 0 0 -2px;
>+    width: 32px;
>+    height: 7px;
>+    background-image: -moz-linear-gradient(rgb(247,182,82), rgb(215,98,10) 95%);
>+    background-clip: content-box;
>+    border: 2px solid;
>+    border-top-width: 0;
>+    -moz-border-right-colors: rgba(255,255,255,.3) rgba(0,0,0,.7);
>+    -moz-border-bottom-colors: rgba(255,255,255,.3) rgba(0,0,0,.7);
>+    -moz-border-left-colors: rgba(255,255,255,.3) rgba(0,0,0,.7);
>+    border-radius: 0 0 3px 3px;
>+  }

My God! What exactly is this stuff doing?
(In reply to comment #245)
> >--- a/browser/themes/winstripe/browser/browser-aero.css
> >+++ b/browser/themes/winstripe/browser/browser-aero.css
> >[...]
> 
> My God! What exactly is this stuff doing?

It draws a window frame around the drag thumbnail.
The ::before style draws a miniature orange appmenu button.
Will add much needed comments before checking in.
(In reply to comment #246)
> (In reply to comment #245)
> > >--- a/browser/themes/winstripe/browser/browser-aero.css
> > >+++ b/browser/themes/winstripe/browser/browser-aero.css
> > >[...]
> > 
> > My God! What exactly is this stuff doing?
> 
> It draws a window frame around the drag thumbnail.

Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame?

> The ::before style draws a miniature orange appmenu button.

Quite a hack, questionable benefit :/
Attached patch patch v15 [ui-r=limi] (obsolete) — Splinter Review
Accounted for OS X panel repositioning.
Fixed a selector on Windows.
Added comments to Windows theming code.

(In reply to comment #247)
> Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame?

Yes. However, it does not allow the elements to be drawn inside the frame to create a more stylized panel such as the one shown here: http://www.stephenhorlander.com/images/blog-posts/tab-animation/animation-tab-tear-off.html
We don't want Firefox to appear generic.

> > The ::before style draws a miniature orange appmenu button.
> 
> Quite a hack, questionable benefit :/

The intentionally recognizable button makes it clear that the panel represents and will "transform" into a new window upon drop.

If we decide on an absolute size for the panel that is independent of window and screen size, then we can easily use -moz-border-image to draw an arbitrary non-overlapping frame.
Attachment #546359 - Attachment is obsolete: true
Attachment #546389 - Flags: review?(dolske)
Attachment #546359 - Flags: review?(dolske)
(In reply to comment #248)
> > Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame?
> 
> Yes. However, it does not allow [...]

Why is it in the stylesheet then?
When dragging a tab out of the window, the tab shown in the browser should change, like in Chrome and IE.
When you drag a tab out of a window where there is only one tab, the window should be hidden (like in Chrome and IE).
The live tab also causes FishIE to go from 60 FPS to something like 10. Not sure whether this is a bug.
Looking good though. Like the window frame, though you may be able to slip "Firefox" inside the orange box.
If you are trying to make the detached tab look like a window, then it would be nice to have it look like a tab , just like chrome makes a detached tab look like a tab.

Also it would be nice if the detached tab could be dragged over the taskbar (in windows) and dock (in mac) . Atl+ TAB is a temporary solution, also there is no proper indication that the usre can Alt+Tab to go to the next maximized firefox window.
Mozilla - I commend you for all the hard work you are putting into this project and for resolving bug issues.  I am rooting for completion within Firefox 8.

I have noticed a minor issue with the tab previews when the sites involve Flash objects and am using the 7/17/11 User Interface Nightly.

If I have www.gsn.com open and drag the tab down to make another window I can see the Flash object (top slideshow with arrows) appear in the mini preview.

However, if I have ANY of the following sites open (Crossword puzzles which use Flash):
  1) http://games.latimes.com/index_crossword.html?uc_feature_code=tmcal
  2) http://www.patsajakgames.com/games/psgcc_online.html
  3) http://www.patsajakgames.com/games/luckyletters_online.html
  4) http://www.flashgames247.com/play/13922.html

I don't see the Flash objects in the mini tab drag preview.

Screenshots are provided:
"Flash Object Missing" = Site 1 above
"Flash Object Appears" = www.gsn.com
Attached image Flash Object Missing (obsolete) —
Attached image Flash Object Appears (obsolete) —
Frank, these latest changes are great. I love the little orange Firefox button in the tear-off window. That adds so much context that I no longer lean towards showing the tab as context in the tear-off like the other browsers do. 

There is really only one (or maybe two) style issue I see left. The glass-like window border on the tear-off window is completely transparent when it should have some opacity or blur or opacity so the content behind it doesn't peek through so strong. The window frame should also picked up the window color from the system (the default on Win7 is "sky" so we could at least fake that if the actual window color isn't available to us via css. Arrow panels currently fake this color, I believe, because it doesn't change when I change window colors.)

Kudos again on the Firefox button. I think that makes a huge difference how the tear off feature feels. It really conveys that what you're dragging out is a window and not some foreign widget.
Frank, sorry about that last comment. I see we're waiting on the color to be available via CSS at bug 578780.  Is there anything we can do in the mean time to fake it like the Arrow panel does. That fakes the color but not the opacity/blur. Is there anything we can do to make the glass-like portions semi-opaque?
OK. I've looked at Arrow panels and this doesn't seem like an super-simple change so let's wait and I'll file follow-up bugs. Let's get what we have here reviewed and landed and deal with the edge cases and style clean-up in new bugs.
Bug:
1. Set the Firefox window to a certain width
2. Drag a tab out, don't release, put it back in the tab bar
3. Reduce Firefox's width

It doesn't reduce beyond the width.
(In reply to comment #249)
> (In reply to comment #248)
> > > Does -moz-appearance: -moz-win-borderless-glass; not draw a window frame?
> > 
> > Yes. However, it does not allow [...]
> 
> Why is it in the stylesheet then?

Using the default -moz-appearance makes the panel opaque. Do you know of a -moz-appearance value that would enable transparency without unnecessary Aero Glass and border effects?

(In reply to comment #252)
> I don't see the Flash objects in the mini tab drag preview.

This is a "bug" in the API I used for live previews (bug 506826). Whether -moz-element is able to draw the plugin's content is dependent on the plugin content's so-called "windowed" state. Plugins are in charge of rendering their portion of the page, so it's non-trivial to resolve.
I'll move the screenshots to a separate bug and ask the platform team what it would take to fix it.

(In reply to comment #255)
> There is really only one (or maybe two) style issue I see left. The
> glass-like window border on the tear-off window is completely transparent
> when it should have some opacity or blur or opacity so the content behind it
> doesn't peek through so strong. The window frame should also picked up the
> window color from the system (the default on Win7 is "sky" so we could at
> least fake that if the actual window color isn't available to us via css.
> Arrow panels currently fake this color, I believe, because it doesn't change
> when I change window colors.)

It's not completely transparent. It's white with an opacity of 0.3. For better or worse, we can't blur content that is not ours. The OS does that as part of its window compositing. Perhaps, we could just make it more frosted, since we can't do the blur.

> Kudos again on the Firefox button. I think that makes a huge difference how
> the tear off feature feels. It really conveys that what you're dragging out
> is a window and not some foreign widget.

Thanks. A shoutout to Felipe, who helped me out with those latest changes.
Why can't we use the blur (and border) from -moz-appearance: -moz-win-borderless-glass? This way the mini window could be constructed roughly the same way, the main window is, and we will be having a nice and native glass effect. This does not work for arrow panels because of the non-rectangular shape (especially the arrow at the top) but I don't see why it wouldn't work here.
> Using the default -moz-appearance makes the panel opaque. Do you know of a
> -moz-appearance value that would enable transparency without unnecessary
> Aero Glass and border effects?

"none"
Comment on attachment 546389 [details] [diff] [review]
patch v15 [ui-r=limi]

Review of attachment 546389 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/winstripe/browser/browser-aero.css
@@ +321,5 @@
> +  }
> +  .tab-drag-preview::before { /* draw miniature appmenu button */
> +    content: "";
> +    display: block;
> +    margin: -14px 0 0 -2px;

Is this going to work correctly in RTL mode too?
Depends on: 672710
Attached patch patch v16 [ui-r=limi] (obsolete) — Splinter Review
Simplified workaround for broken z-order enumerator on Linux.

(In reply to comment #247) (Dão)
(In reply to comment #260) (Jonathan)

I switched to using the native glass effect, but there's a bug with the border and box shadow. I filed bug 672710 for that.

(In reply to comment #262)
> Is this going to work correctly in RTL mode too?

No, but I fixed it in this version. Thanks for catching it.
Attachment #546389 - Attachment is obsolete: true
Attachment #546424 - Attachment is obsolete: true
Attachment #546425 - Attachment is obsolete: true
Attachment #546965 - Flags: review?(dolske)
Attachment #546389 - Flags: review?(dolske)
Blocks: 638488
No longer depends on: 672710
Depends on: 458864
The thumbnail has an unneeded grey square corner outside the bottom-right corner of the aero border. http://tinyurl.com/3n8znwe
Also, the animations are little bit sluggish and the tabs stuck for a little time when moved first.
I'm sorry, first is Bug 458864...
Attached patch patch v17 [ui-r=limi] (obsolete) — Splinter Review
Moved generic Windows style rules from browser-aero.css to browser.css.
Optimized code that iterates through windows front-to-back.

Not yet obsoleting patch v16 in case dolske already started reviewing it.
Attachment #547135 - Flags: review?(dolske)
(In reply to comment #267)
> from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7

I watched the video about five times.....what exactly are we looking at / the issue here??
Yep, better communication please. At first look I saw a blank tab before app tabs.
(In reply to comment #268)
> (In reply to comment #267)
> > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7
> 
> I watched the video about five times.....what exactly are we looking at /
> the issue here??

A non-pinned tab preceding pinned tabs.

(In reply to comment #269)
> Yep, better communication please.

Certainly a great example for good communication, that sentence.
I guess the unspoken question was 'how did that happen?'

Anyway, here's the link to the Mozillazine posts referenced earlier. Might not help entirely, but at least there's a clearer picture there; http://forums.mozillazine.org/viewtopic.php?f=23&t=2230295&start=263
the detach-preview-window (or whatever its called technically) now uses aero blurred transparency , but dragging it is quite slow compared to dragging original firefox window.
(In reply to comment #267)
> from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7

(In reply to comment #271)
> I guess the unspoken question was 'how did that happen?'

After watching it five times, I still don't understand how to reproduce this. The linked Mozillazine thread seems to be discussing tab duplication, which is unrelated.

(In reply to comment #272)
> the detach-preview-window (or whatever its called technically) now uses aero
> blurred transparency , but dragging it is quite slow compared to dragging
> original firefox window.

What does "original firefox window" mean in this context?
(In reply to comment #273)
> (In reply to comment #267)
> > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7
> 
> (In reply to comment #271)
> > I guess the unspoken question was 'how did that happen?'
> 
> After watching it five times, I still don't understand how to reproduce
> this. The linked Mozillazine thread seems to be discussing tab duplication,
> which is unrelated.

quote:

- http://cl.ly/8cxW/Image_2011-07-20_at_8.10.38_PM.png - Google+ was not an app tab. This occurred after going into panorama. I can't reproduce it; I'm really not sure how it happened
- Yup, saw it multiple times. Happens when you're quickly dragging stuff
(In reply to comment #274)
> > (In reply to comment #267)
> > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7
>
> [...] Happens when you're quickly dragging stuff

I've never seen it happen.
Any idea why it happens? moveTabTo() is supposed to guard against that.
Have you tried it with the latest UX build?
(In reply to comment #275)
> (In reply to comment #274)
> > > (In reply to comment #267)
> > > > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7
> >
> > [...] Happens when you're quickly dragging stuff
> 
> I've never seen it happen.
> Any idea why it happens? moveTabTo() is supposed to guard against that.

I would think so.

> Have you tried it with the latest UX build?

In case you were asking me specifically: I haven't seen it myself. (Haven't tried to reproduce it either.)
(In reply to comment #276)
> (In reply to comment #275)
> > Any idea why it happens? moveTabTo() is supposed to guard against that.
> 
> I would think so.

In that case, they are probably using some add-on. If an add-on is writing their own poor implementation of moveTabTo(), that can't be helped. There have already been "bug reports" in this bug that are actually caused by Tab Utilities.
(In reply to comment #277)
> (In reply to comment #276)
> > (In reply to comment #275)
> > > Any idea why it happens? moveTabTo() is supposed to guard against that.
> > 
> > I would think so.
> 
> In that case, they are probably using some add-on. If an add-on is writing
> their own poor implementation of moveTabTo(), that can't be helped. There
> have already been "bug reports" in this bug that are actually caused by Tab
> Utilities.

(Person that posted the image of the Google+ Tab bug here)

This was the most recent UX Build when I posted (late yesterday). 

My addons include:
DOM Inspector, Firebug (1.7), Flashblock, Garmin Communicator, Greasemonkey, Growl For Windows/GNTP, Missing e (Tumblr modifier), Prospector Instant Preview, Prospector Speak Words, Stylish (you can see my customizations in the screenshot), Twitter Address Bar Search, and User Agent RG.

The only one of this that I think might impact tabs would be Stylish, but since I can't reproduce reliably, I can't test it
(In reply to comment #278)
> (Person that posted the image of the Google+ Tab bug here)

Thanks for commenting.

> This was the most recent UX Build when I posted (late yesterday). 

I pushed out a change yesterday, so today's build is slightly different, although, this problem should have never appeared in any version of the bug's patch.

> My addons include:
> DOM Inspector, Firebug (1.7), Flashblock, Garmin Communicator, Greasemonkey,
> Growl For Windows/GNTP, Missing e (Tumblr modifier), Prospector Instant
> Preview, Prospector Speak Words, Stylish (you can see my customizations in
> the screenshot), Twitter Address Bar Search, and User Agent RG.
> 
> The only one of this that I think might impact tabs would be Stylish, but
> since I can't reproduce reliably, I can't test it

I doubt Stylish is causing the problem. I don't know about the other add-ons.
(In reply to comment #279)
> I doubt Stylish is causing the problem. I don't know about the other add-ons.


I agree, but it seems most likely out of the others, as the rest don't impact tabs at all. I have a few modifications on tabs in stylish. Would making the margin of .tabbrowser-tab -2px impact the tab location detection so that this could happen?

Otherwise, I'm not sure it was an addon. I can't reproduce the quickly dragging tabs to force one tab over like the video, I got this to occur when switching from Panorama, so I'm not sure how related the two are
(In reply to comment #273)
> (In reply to comment #267)
> > from mozillazine: http://v7.tinypic.com/player.swf?file=25g5tmf&s=7
> 
> (In reply to comment #271)
> > I guess the unspoken question was 'how did that happen?'
> 
> After watching it five times, I still don't understand how to reproduce
> this. The linked Mozillazine thread seems to be discussing tab duplication,
> which is unrelated.
> 
> (In reply to comment #272)
> > the detach-preview-window (or whatever its called technically) now uses aero
> > blurred transparency , but dragging it is quite slow compared to dragging
> > original firefox window.
> 
> What does "original firefox window" mean in this context?


i just mean to say that the detach thumbnail window is slow at dragging. (Original firefox window meant the whole browser window)
Comment on attachment 547135 [details] [diff] [review]
patch v17 [ui-r=limi]

Review of attachment 547135 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to comment #243)
> (In reply to comment #242)
> > In windows too
> 
> After checking other people's experiences on the mozillazine UX branch
> thread ([1]), it ended up being an extension's fault.  It's likely to be the
> same for you, if you use Tab Utilities or a similar addon.  WFM on a clean
> profile.
> 
> [1]: http://forums.mozillazine.org/viewtopic.php?p=11027729#p11027729

::: browser/base/content/tabbrowser.xml
@@ +2983,5 @@
> +
> +      <method name="_updateTabDetachState">
> +        <parameter name="event"/>
> +        <body><![CDATA[
> +          let draggedTab = this.selectedItem;

The patch is assuming that people are always dragging the active tab. This is the fault.
(In reply to comment #282)
> The patch is assuming that people are always dragging the active tab.

Yes, the dragged tab should always be the active one, since tabs are selected on mousedown. Dragging background tabs and select on mouseup/click are not interactions we plan to support.
(In reply to comment #283)
> (In reply to comment #282)
> > The patch is assuming that people are always dragging the active tab.
> 
> Yes, the dragged tab should always be the active one, since tabs are
> selected on mousedown. Dragging background tabs and select on mouseup/click
> are not interactions we plan to support.

We all understand that Firefox won't support all options by itself, but you should not be aiming at creating inabilities for extensions. You'd better get the dragged tab from some pass-in parameter instead of assuming the active tab. Another logical solution is to save the dragged tab in gBrowser.mContextTab.
Attached patch patch v18 [ui-r=limi] (obsolete) — Splinter Review
Had to unbitrot patch due to the landing of the patch for bug 649480.
For the sake of add-on compatibility, removed assumption that dragged tab is active tab. Fortunately, this comes with zero additional maintenance cost.
Attachment #547614 - Flags: review?(dolske)
Comment on attachment 547614 [details] [diff] [review]
patch v18 [ui-r=limi]

>+#main-window[privatebrowsingmode=temporary] #tabbrowser-tabs > hbox > .tab-drag-panel > .tab-drag-preview::before,

What is #tabbrowser-tabs > hbox > .tab-drag-panel > doing here?
I think drawing a windows aero-glass border outside the preview of tab is cool, but its aim is to show the preview, the fav-icon, the title of the holding tab. So don't draw Firefox button; just the border, title and fav-icon is enough for the user to understand that they are creating a new Firefox window with that page.
It would be nice if you could press [esc] to cancel detaching the tab.

I've also noticed that occasionally my cursor will exit the preview pane and I can't get the tab to nest itself into the original window; releasing the mouse button has no effect as the preview window keeps moving with the cursor. This must sound pretty vague, so let me know if I need to make a screencap.
(In reply to comment #288)
> It would be nice if you could press [esc] to cancel detaching the tab.
> 
> I've also noticed that occasionally my cursor will exit the preview pane and
> I can't get the tab to nest itself into the original window; releasing the
> mouse button has no effect as the preview window keeps moving with the
> cursor. This must sound pretty vague, so let me know if I need to make a
> screencap.

Yup , I can confirm that , its exactly like you say.

Sometimes when trying to detach the tab  , it does not tear off. (Now the mouse is away from the tab strip as we tryed to detach the tab). Now we bring the mouse back to tab strip and the tab is magically pinned to the mouse pointer like we are still dragging it around (even if there is no mouse down) . Now unless we double click the tab strip , the tab does not let go off the mouse.
I've noticed an issue (not at all serious, mind you) with two monitors on Windows 7, where dragging the detached tab to the edge of the screen sort of sticks until the mouse crosses over at which point it jumps to the other screen, so the tab preview panel won't appear on both screens. Like I said, not particularly problematic, just looks a little twitchy.
I like where the UX build is at now, but believe it still needs to be polished a bit before prime time.  I am running Windows 7 64-Bit with 4 GB of RAM and an EVGA GeForce GTS450 (PCI-E 2.0) containing 1 GB of GDDR5 memory and notice sluggishness moving tabs around after having only 12 or 13 tabs open.
Attachment #546965 - Attachment is obsolete: true
Attachment #546965 - Flags: review?(dolske)
Attachment #547135 - Attachment is obsolete: true
Attachment #547135 - Flags: review?(dolske)
Comment on attachment 547614 [details] [diff] [review]
patch v18 [ui-r=limi]

Review of attachment 547614 [details] [diff] [review]:
-----------------------------------------------------------------

1) [Note to self...] The two issues I raised in the previous review are sufficiently addressed now...

One was the issue of how to detect when an intermediate, non-Mozilla window is above a background Firefox window (such that if you got unlucky, your fling to detach a tab into a new window actually move it to the background window, and you're left wondering what happened). That's made moot by raising the background window, so that if you move a tab that way the window you've dragged to should always end up on top.

The other was the issue with the broken getZOrderDOMWindowEnumerator. It's ok on Windows, bug 450576 now indicates it seems to be working on OS X, and although it's still broken on Linux (bug 156333), I'm satisfied that (1) that bug is getting activity again and (2) the workaround in this patch is sufficient to reduce the user impact to just an annoyance. Specifically, the impact is that the workaround may, sometimes, raise a window that although is correctly under your mouse pointer, is actually behind the window you see. But from some experimentation that's still generally ok -- you already have to carefully aim for the other window's tabstrip to move a tab there, and so even if the wrong window pops forward you're still likely to be concentrating on the window you were aiming for, and if it's sticking out you can still drag over to that to bring it forward and successfully complete the task. Obviously not _great_, but acceptable for now. Additionally, even on platforms with a working getZOrderDOMWindowEnumerator() there are pathological cases where we "correctly" raise a window and make it difficult to get at the window you meant to drag to. Long story short -- still some tweaking to do but the gain is far, far, greater than the pain at this point.

2) Tests. Between soaking on the UX branch recently, a year's worth of test builds, and existing test coverage I'm not too concerned. But since there have been some subtle (but carefully engineered) behaviors added in this patch, it would be good for future sanity to avoid unknowingly breaking them. I'd suggest filing a followup bug with a written description of what you think is good to check for. That allows you/us to have a braindump of what you know to look for now, and serves as a guideline for future testing... Ideally as automated tests, maybe even MozMill tests.

Anyway, a few small nits in this review... Fix those + test followup and OMG LAND IT! :)

::: browser/base/content/tabbrowser.css
@@ +38,5 @@
>    background-color: transparent;
>  }
>  
> +.tab-drag-preview {
> +  background: -moz-element(#content) left top;

I suppose this won't show the right thing when dragging a background tab?

::: browser/base/content/tabbrowser.xml
@@ +2831,5 @@
> +          event.preventDefault();
> +          event.stopPropagation();
> +
> +          var tabStrip = this.mTabstrip;
> +          var ltr = (window.getComputedStyle(this, null).direction == "ltr");

Would |ltr = (window.document.dir == "ltr");| work here?

This gets called fairly frequently, so it should be cheaper. Also, getComputedStyle is going to do a style flush, which could be a bunch of work and cause lag (though for all I know we might be causing that elsewhere too, so eliminating this one might not matter (yeah, it looks like we do)). Hmm, seems like this might be interesting fodder for some benchmarking / metrics. :)

@@ +2944,5 @@
> +          let numPinned = this.tabbrowser._numPinnedTabs;
> +          let leftmostTab = draggedTab.pinned ? tabs[0] : tabs[numPinned];
> +          let rightmostTab = draggedTab.pinned ? tabs[numPinned-1] : tabs[tabs.length-1];
> +          let tabWidth = draggedTab.getBoundingClientRect().width;
> +          let ltr = (window.getComputedStyle(this).direction == "ltr");

Ditto, as above.

@@ +2994,5 @@
> +            draggedTab._targetWindow = window;
> +            window.focus();
> +          }
> +          else if (draggedTab._dropTarget)
> +            draggedTab._dropTarget._tabDropIndicator.collapsed = true;

Please brace else-block since the former if-block was braced.

@@ +2995,5 @@
> +            window.focus();
> +          }
> +          else if (draggedTab._dropTarget)
> +            draggedTab._dropTarget._tabDropIndicator.collapsed = true;
> +          delete draggedTab._dropTarget;

Is this supposed to be part of the else-block? Seems odd to be nuking it every call...

@@ +2997,5 @@
> +          else if (draggedTab._dropTarget)
> +            draggedTab._dropTarget._tabDropIndicator.collapsed = true;
> +          delete draggedTab._dropTarget;
> +
> +          function isEventOutsideWindow(event, win)

Please brace body to make it stand out better.

Can any of the other similar "is x bounded by y" checks below make use of this too? Especially if you passed in window.boxObject instead of window?

@@ +3007,5 @@
> +            let winEnum = Services.wm.getZOrderDOMWindowEnumerator("navigator:browser", true);
> +            let wins = [];
> +            while (winEnum.hasMoreElements())
> +              wins.push(winEnum.getNext());
> +            // Work around broken z-order enumerator on Linux.

Please note bug number here.

@@ +3013,5 @@
> +              winEnum = Services.wm.getEnumerator("navigator:browser");
> +              while (winEnum.hasMoreElements())
> +                wins.unshift(winEnum.getNext());
> +            }
> +            wins.every(function(win) {

Might it be clearer to use .some() instead of .every()? Or better yet just a plain old loop + break/continue?

@@ +3049,5 @@
> +            else
> +              dragPanel.removeAttribute("target");
> +
> +            if (!detached) {
> +              this.setAttribute("drag", "detach");

Set |detached = true| here? It's not used again, but lest someone add more code later...

@@ +3062,5 @@
> +              let preview = dragPanel.lastChild;
> +              label.value = draggedTab.label;
> +              label.width = preview.width = Math.min(outerWidth / 2, screen.availWidth / 5);
> +              let aspectRatio = this.tabbrowser.clientWidth / this.tabbrowser.clientHeight;
> +              preview.height = Math.min(preview.width / aspectRatio, screen.availHeight / 5);

Do you want a Math.round() / Math.floor() here, to ensure the divisions are integers?

@@ +3102,5 @@
> +            .screenForRect(aLeft, aTop, 1, 1)
> +            .GetAvailRect(sX, sY, sWidth, sHeight);
> +          if (isPanel) { // Avoid getting repositioned by the window manager.
> +            sWidth.value -= 3;
> +            sHeight.value -= 3;

Why 3?

@@ +3254,5 @@
> +          delete tab._maxWidth;
> +          delete tab._savedEvent;
> +          delete tab._targetWindow;
> +          delete tab._dropTarget;
> +

Hmm. Is this is to avoid leaking/bloat, and/or to make sure code doesn't thing we're mid-drag due to a stale property leftover from the last drag?

There are enough of these that I wonder if it would be safer to stuff everything into a tab.dragData object. You would then just null it out here (or set it to a new, empty object), and if we add more bits to dragData later there's no risk of forgetting to clear them here.

@@ +3526,4 @@
>                  break;
> +              let bo = this.mTabstrip.boxObject;
> +              if (aEvent.screenX >= bo.screenX && aEvent.screenX < bo.screenX + bo.width &&
> +                  aEvent.screenY >= bo.screenY && aEvent.screenY < bo.screenY + bo.height)

Yet another of these! :)
Attachment #547614 - Flags: review?(dolske) → review+
(In reply to comment #292)
> > +  background: -moz-element(#content) left top;
> 
> I suppose this won't show the right thing when dragging a background tab?

Yes, so I added back in a TabSelect handler, which Dolske rs'd.
Add-ons will need to restyle the panel when the tab being dragged is not selected. This can be done using TabSelect and dragstart handlers that set an attribute on .tab-drag-panel and including something like the following in a stylesheet:
.tab-drag-panel:not([selected]) { /* use something besides -moz-element */ }

> > +          var ltr = (window.getComputedStyle(this, null).direction == "ltr");
> 
> Would |ltr = (window.document.dir == "ltr");| work here?

No, AFAICT, document.dir always returns "ltr" and does not necessarily reflect whether the tab strip is RTL.

> > +          delete draggedTab._dropTarget;
> 
> Is this supposed to be part of the else-block? Seems odd to be nuking it
> every call...

It's okay to delete it, because we have to check what the value needs to be with every mousemove anyway.

> Can any of the other similar "is x bounded by y" checks below make use of
> this too? Especially if you passed in window.boxObject instead of window?

No, window.boxObject is undefined.

> > +            if (!detached) {
> > +              this.setAttribute("drag", "detach");
> 
> Set |detached = true| here? It's not used again, but lest someone add more
> code later...

I don't think that's necessary or even useful. |detached| is more of a "wasDetached", and it's more likely that someone would want to know the previous value.

> > +          if (isPanel) { // Avoid getting repositioned by the window manager.
> > +            sWidth.value -= 3;
> > +            sHeight.value -= 3;
> 
> Why 3?

Empirical data.

> > +          delete tab._maxWidth;
> > +          delete tab._savedEvent;
> > +          delete tab._targetWindow;
> > +          delete tab._dropTarget;
> > +
> 
> Hmm. Is this is to avoid leaking/bloat, and/or to make sure code doesn't
> thing we're mid-drag due to a stale property leftover from the last drag?

The former.

> There are enough of these that I wonder if it would be safer to stuff
> everything into a tab.dragData object.

Done.

Carrying r+ forward. Will land soon.
Additional followups will be filed shortly as well.
Attachment #547614 - Attachment is obsolete: true
Pushed to m-c and also merged onto ux.

https://hg.mozilla.org/mozilla-central/rev/9d4e815ae581
https://hg.mozilla.org/mozilla-central/rev/956bc17951c2
https://hg.mozilla.org/mozilla-central/rev/048afee8ae1e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Depends on: 156333
Blocks: 674487
No longer blocks: 674487
Depends on: 674487
I am not sure, but I see a little bit "heavier" animations when using Firefox in windowed mode. In maximized mode it's quite smooth. Probably I see difference because my CPU is slow (netbook Atom N450). I can't reproduce this on desktop machine (quad core).
Win 7 Pro SP1 32bit, fresh profile, hourly build from http://hg.mozilla.org/mozilla-central/rev/e4c8a1e7b373
got this bug:
http://img828.imageshack.us/img828/3423/bugnj.png

detach tab...attach it again (over another tab)...repeat..bug happens
got it with 2 tabs and with with many tabs also (5 tabs)
Depends on: 674732
(In reply to comment #298)
> Fixed typo.
> 
> https://hg.mozilla.org/mozilla-central/rev/d066929dd830

So this looks like a code change.  Why would it be marked DONTBUILD?  Should not tests be re-run?
No longer depends on: 674732
Restoring dependency removed by previous comment/mid-air.
Depends on: 674732

Dragged tab seems to freeze and lag when alt-tabbing to a different window.

Make a new Nightly window 
Go back to your currently used window
Detach one tab 
alt-tab to the newly made window 
Try to move the tab to the tab area

The dragged tab will freeze a bit below half the screen and be unresponsive, it needs to be dropped to make its own window to make any use of it. 

Here's a problem, alt-tabbing when the detached tab is close to the chrome poses no problem, it can be moved whenever I like, but if the detached tab is moved a bit below the chrome it becomes unresponsive after alt-tab.

Should I make a new bug?
(In reply to comment #301)

No. Bug 674487.
I have a little Problem with Windows 7: You cannot drag a detached Tab to the Task bar to bring a (hidden or minimized) firefox window up.

In Windows 7 you can usually drag things over the icon in the taskbar to get minimized or hidden windows to the front.
First, woot for finally getting this bug landed. Kudos to fryn for sticking through it over the last year. ;-)

Secondly, this bug is now closed. If you have problems or issues, please file NEW BUGS and mark them as blocking this one. There are 303 comments and 167 people CC'd to this bug -- further comments here just annoy people are are likely to get lost and not acted upon.

Repeat: File NEW BUGS for any issues. We're done here. Thanks!
Depends on: 674789
Depends on: 674815
Depends on: 674819
Depends on: 674823
(In reply to comment #303)
> I have a little Problem with Windows 7: You cannot drag a detached Tab to
> the Task bar to bring a (hidden or minimized) firefox window up.
> 
> In Windows 7 you can usually drag things over the icon in the taskbar to get
> minimized or hidden windows to the front.

Please file a bug on this.
Depends on: 674825
Depends on: 674831
Filed Bug 674831. We still need a bug about tabs dragged between windows not appearing as real tabs.
Depends on: 674833
No longer depends on: 674833
Depends on: 674847
Depends on: 674840
Blocks: 674853
No longer depends on: 674825
Comment on attachment 548845 [details]
Improving the design of preview the detached tab.

We do not necessarily know what the user's window caption buttons (min/max/close) look like, so this design is not necessarily more accurate nor more useful than the current one.
Attachment #548845 - Attachment is obsolete: true
No longer depends on: 674840
No longer blocks: 674853
Depends on: 674853
No longer blocks: 579629
Depends on: 674840
(In reply to comment #307)
> Comment on attachment 548845 [details]
> Improving the design of preview the detached tab.
> 
> We do not necessarily know what the user's window caption buttons
> (min/max/close) look like, so this design is not necessarily more accurate
> nor more useful than the current one.

Also user can have disabled app-button (:
This line removal got lost somehow.
Text editors and I need to become better friends.
https://hg.mozilla.org/mozilla-central/rev/fec9d1d409cf
Depends on: 674913
Depends on: 674940
Depends on: 675340
Depends on: 675440
Can we get an about:config setting to disable the animations? It breaks tab dragging in TMP and currently has conflicts with the task bar.
Depends on: 675451
(In reply to comment #310)
> Can we get an about:config setting to disable the animations? It breaks tab
> dragging in TMP and currently has conflicts with the task bar.

I've added bug 675451 which is aimed at that goal.
Depends on: 675860
Depends on: 675863
Depends on: 675209
Hello,

First, the tab animation is very good and the detach animation too.
But I've found a small graphic bug in the detach animation : the bottom-right corner has not a correct shadow.
Maybe, It happens only on my computer (W764bit, Intel Core 2 Duo, ASUS Nvidia 460 GTX).
Attached image A small bug of the detach animation. (obsolete) —
(In reply to comment #313)
> Created attachment 550222 [details]
> A small bug of the detach animation.

That would be Bug 458864.
Comment on attachment 550222 [details]
A small bug of the detach animation.

(In reply to comment #313)
> Created attachment 550222 [details]
> A small bug of the detach animation.

Read comment 264, comment 265,and comment 304.
Bug 458864 is a known issue.
Attachment #550222 - Attachment is obsolete: true
Depends on: 676304
After landing this bug, tabs can't be bookmarked by dropping them into the bookmarks toolbar. Now you need to press CTRL first. Is this new behavior set in stone or subject to discuss?
(In reply to comment #316)
> After landing this bug, tabs can't be bookmarked by dropping them into the
> bookmarks toolbar. Now you need to press CTRL first. Is this new behavior
> set in stone or subject to discuss?

Is it related to bug 674723?
(In reply to comment #317)
> (In reply to comment #316)
> > After landing this bug, tabs can't be bookmarked by dropping them into the
> > bookmarks toolbar. Now you need to press CTRL first. Is this new behavior
> > set in stone or subject to discuss?
> 
> Is it related to bug 674723?

Sorry. This comment is wrong.
this bug is FIXED, take your discussions to 
http://groups.google.com/group/mozilla.dev.apps.firefox/topics?lnk=rgh

File follow up bugs blocking this one if you have problems/issues with the current patch/implementation.

thanks
can I have the arrow indicator back? horrible 'animation'
(In reply to dindog from comment #320)
> can I have the arrow indicator back? horrible 'animation'

Watch bug 675451
Depends on: 676686
Blocks: 649671
I don't want this stinking animation.

I want to be able to Drag and Drop my Tabs into the Bookmark Toolbar.
(In reply to Malcolm from comment #322)
> I don't want this stinking animation.
> 
> I want to be able to Drag and Drop my Tabs into the Bookmark Toolbar.

Atleast try to read the previous comments , you are aware of the fact that your repetitive  comment has bothered around 150 people .
And for your answer : You can drag your tab to the bookmarks toolbar by holding Ctrl .
What's that Firefox Slogan advertisement on the Billboards:

"Every browser does fast. But not every browser does good."

After Firefox 4, there is just an added hassle to get the features I want.

First I had to go to Options and enable the Menu Bar.

After that I had to go to View ==> Toolbars and enable the Bookmarks Toolbar.

Then I had to Uncheck ==> Tabs On Top.

Then I had to click View ==> Toolbars ==> Customize to get the Home Screen & Stop Button in the proper location.

And then Firefox changed the location of the "Open in New Tab" when we do right click. Instead of opening in new tab I kept opening in new window (like IE6). I finally got used to it now.

Coming to that topic why did you not remove "Open in New Window" while you went around gutting all the features for Firefox 4 and making it so much harder to use Firefox.

And now coming soon to Firefox 9 ==> More hassles, starting with pressing CTRL when bookmarking a tab, and who knows what else the UI teams wants to break.

Plus new UI means more bloat, regression bugs, and also I am sure more security vulnerabilities.

Lets change this slogan to:

"Every browser does fast. No browser does good."
(In reply to Malcolm from comment #324)
What in the world does any of this have to do with the tab reorder animation???? Please don't spam bugs.
Whatever you think you're trying to accomplish by ranting in a closed bug, Malcolm, you're not accomplishing it. You are, however, emailing 163 people every time you try.

If you have constructive suggestions for changes, and can express them without insulting the people who try very hard to build a good browser, please file new bugs.
from	Girish Sharma scrapmachines@gmail.com
to	malcolm.turmel@gmail.com
date	Tue, Aug 23, 2011 at 7:40 AM
subject	Re : [Bug 455694] Implement animation for tab reordering and detaching (via drag and drop)
	
hide details 7:40 AM (5 minutes ago)
	
(In reply to Malcolm from comment #324)
> What's that Firefox Slogan advertisement on the Billboards:
>
> "Every browser does fast. But not every browser does good."
>
> After Firefox 4, there is just an added hassle to get the features I want.
>
> First I had to go to Options and enable the Menu Bar.
>
> After that I had to go to View ==> Toolbars and enable the Bookmarks Toolbar.
>
> Then I had to Uncheck ==> Tabs On Top.
>
> Then I had to click View ==> Toolbars ==> Customize to get the Home Screen &
> Stop Button in the proper location.
>
> And then Firefox changed the location of the "Open in New Tab" when we do
> right click. Instead of opening in new tab I kept opening in new window
> (like IE6). I finally got used to it now.
>
> Coming to that topic why did you not remove "Open in New Window" while you
> went around gutting all the features for Firefox 4 and making it so much
> harder to use Firefox.
>
> And now coming soon to Firefox 9 ==> More hassles, starting with pressing
> CTRL when bookmarking a tab, and who knows what else the UI teams wants to
> break.
>
> Plus new UI means more bloat, regression bugs, and also I am sure more
> security vulnerabilities.
>
> Lets change this slogan to:
>
> "Every browser does fast. No browser does good."

Ohk , we get it , you are that lazy bum who can't even cope up with the slow changes of the world, and who just wants things the old retarded way, smoking out his pipe , laying back on drugs , and just liking the browser as it was ages ago , howsoever new and better technology arrives , he just wants his **** old **** ways to carry on.
I think you get my pint .
Also , please don't spam that bug anymore.
Have mercy on other 163 persons following that bug.
Stop venting you **** frustration here on this bug.

-- 
Girish Sharma
Final Year Student
Civil Engineering Department
IIT Kharagpur
srsly, we are not interested in your internet social relations...
Please go to mozilla.dev.usability newsgroup to discuss your product usability concerns and leave your feedback in a constructive way.
Bug 681368 filed for ctrl-click change.
Depends on: 682559
Blocks: 682559
No longer depends on: 682559
Malcolm's account has been disabled. That sort of abuse is not acceptable in Bugzilla. Please report it if you see it.

Gerv
For tabs which have a PDF opened (using a plugin) no preview appears in the dragged out widget. No preview appears in Panorama also. Is there a tracking bug for the missing previews?
Whiteboard: [parity-safari][parity-chrome][parity-ie] → [parity-safari][parity-chrome][parity-ie][qa+]
No longer blocks: 649671
Depends on: 649671
Blocks: 684741
Depends on: 690227
Depends on: 690341
verified [testday-20110930]
verified Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20100101 Firefox/8.0
verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20100101 Firefox/8.0b1
Verified fixed also with 8.0b1 Linux x86_64 eo.
Status: RESOLVED → VERIFIED
Whiteboard: [parity-safari][parity-chrome][parity-ie][qa+] → [parity-safari][parity-chrome][parity-ie][qa+][testday-2011030]
Whiteboard: [parity-safari][parity-chrome][parity-ie][qa+][testday-2011030] → [parity-safari][parity-chrome][parity-ie][qa+][testday-20110930]
Depends on: 690991
No longer depends on: 690991
Depends on: 691290
Verified also using the latest Nightly, the latest Aurora and Firefox 8.0b1 on Windows XP, Windows 7, Ubuntu 11.04 and Mac OS X 10.6. Animation for tab reordering and detaching is implemented as in the prototype from Comment 12 with the exception that the tabs can't be dragged past the left and right edges of the screen (as mentioned in Comment 99).

In the prototype from Comment 12 beside Move and Detach tabs there are 2 more tabs: Close and New Tab. Are Close and New Tab supposed to be implemented by this path also?
Depends on: 693279
Depends on: 675900
Depends on: 693570
Depends on: 693827
Depends on: 694085
Depends on: 694133
Depends on: 694861
No longer depends on: 694861
Was this a partial back-out ? 
https://hg.mozilla.org/mozilla-central/rev/4bb7c983d589 

should it be re-opened ?
It was not backed out of mozilla-central so it should not be reopened yet. Once it is, this bug is unfixed and needs to be reopened.
(In reply to Christian Legnitto [:LegNeato] from comment #338)
> It was not backed out of mozilla-central so it should not be reopened yet.
> Once it is, this bug is unfixed and needs to be reopened.

then I'm confused - the cset in comment #337 clearly shows m-c ?
Yep, my bad, missed the m-c part.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Yes, we backed it out.
I don't think we should keep this bug reopened.
The thread is long enough.
In the past, we've not reopened many bugs where things were landed, effectively backed out, and re-landed later in a vastly different form.

Either way, I'm not going to do development in this bug.
Development of the new, better implementation will happen in bug 674925, and this bug can be re-closed when the patch there lands.
Status: REOPENED → UNCONFIRMED
Ever confirmed: false
Whiteboard: [parity-safari][parity-chrome][parity-ie][qa+][testday-20110930] → [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie][qa+][testday-20110930]
Status: UNCONFIRMED → NEW
Ever confirmed: true
Probably best to dupe this over to the new development bug. Keeping this open as new if it won't see a patch is pointless.
(In reply to Paul [sabret00the] from comment #343)
> Probably best to dupe this over to the new development bug. Keeping this
> open as new if it won't see a patch is pointless.

Which is?
Yeah, we're done here. This specific implementation has been backed out, and will be reimplemented over in bug 674925 as Frank notes in comment 341.

There are enough people CC'd here and related bugs  that I'm not to dupe this over. Feel free to CC yourself to bug 674925 if you're interested.

WONTFIX for this specific implementation.
Status: NEW → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → WONTFIX
What is it? Like a year of rather active work on it? And now dropped. No wonder firefox keeps falling behind.
(In reply to avada from comment #346)
> What is it? Like a year of rather active work on it? And now dropped. No
> wonder firefox keeps falling behind.

There are problems with this form of implementation with almost All tab-related add-ons. For more info, see Bug 690227. Hence, it is going to be reimplemented in bug 674925.
(In reply to Justin Dolske [:Dolske] from comment #345)
> There are enough people CC'd here and related bugs  that I'm not to dupe
> this over. Feel free to CC yourself to bug 674925 if you're interested.

Marking a bug as a duplicate doesn't copy the CC list, unless this changed recently, which I hope isn't the case...
Resolution: WONTFIX → DUPLICATE
No longer depends on: 693827
Keywords: uiwanted
Whiteboard: [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie][qa+][testday-20110930] → [backed out in bug 690227 -- see bug 674925][parity-safari][parity-chrome][parity-ie]
Assignee: fyan → nobody
No longer blocks: 566510
You need to log in before you can comment on or make changes to this bug.