690227 - Back out bug 455694 (tab drag/detach animations)

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[ithinc]

Bug 455694 regresses many Firefox features, AND it is a deal breaker for quite a few tab-related extensions.

Comment 1

[Dão Gottwald [:dao]]

Do you have some concrete data on the impact on extensions?

Comment 2

[YUKI "Piro" Hiroshi]

Actually, I decided that I don't update some my addons for Firefox 8 until the bug 674925 (or fatal regressions caused by the bug 455694) become resolved. For example:

 * Tree Style Tab https://addons.mozilla.org/firefox/addon/tree-style-tab/
 * Multiple Tab Handler https://addons.mozilla.org/firefox/addon/multiple-tab-handler/
 * Fox Splitter https://addons.mozilla.org/firefox/addon/fox-splitter/

There are two reasons.

1) If the bug 674925 becomes fixed after I updated them for current Firefox 8 (based on plain mouse events), they become obsolete soon. Then I have to update them again for the new Firefox 8. It is too painful.

2) Even if I update them for the current Firefox 8, many regressions from the bug 455694 are still there. I'm worrying that I receive annoying bug reports from those regressions. Yes, of course I can "fix" them on my addons, but it is a bad way, because such ad-hock patches will break compatibility with other addons.

Comment 3

[YUKI "Piro" Hiroshi]

Sorry, I forgot to write how this issue affects my addons.

Tree Style Tab provides a vertical tab bar with tabs like a nested tree, and ability to attach/detach sub-trees of tabs via HTML5 drag and drop events. This issue completely breaks the feature. TST cannot indicate the drop marker for tabs, and it also cannot attach/detach sub-trees. (Moreover, the appearance of the vertical tab bar is broken by animation effects for horizontal tab bar. I think I should fix the animation effects for the vertical tab bar in future versions.)

Multiple Tab Handler provides ability to drag and drop multiple tabs at once. This feature is based on HTML5 drag and drop events5, so, this issue completely breaks the compatibility for Firefox 8.

Fox Splitter provides ability to glue and operate multiple Firefox windows, like split panes in a large window. To do it, you only have to drag a tab and drop on a edge of a Firefox window, then the tab will become a new browsing window and glued to the drop target window automatically. However, because this behavior is implemented based on HTML5 drag and drop events, it doesn't work on Firefox 8.

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I want to add that even simple merging of maximized windows isn't possible now.

Comment 5

[dindog]

(In reply to Dão Gottwald [:dao] from comment #1)
> Do you have some concrete data on the impact on extensions?

Besides the addons mentioned above, ithicn's tabs' stack feature in Tab Utilities 1.2pre is one of those outstanding implements rely on that.

Bug 455694 was there roughly 3 years, we don't have to land it in a rush just because it is on the ROADMAP even it's not well implement.

Comment 6

[Dão Gottwald [:dao]]

Attachment: backout patch (beta)

Landing bug 674925 on mozilla-beta, assuming it had a patch, sounds too risky, so here's the backout patch.

Comment 7

[Mozilla RelEng Bot]

Try run for 66d6c122f3c6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=66d6c122f3c6
Results (out of 15 total builds):
    success: 13
    warnings: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-66d6c122f3c6

Comment 8

[Cattleya]

Hope Firefox developer remove tab drag/detach animation Fx8, it is a awful improvement, it break so much good extension, Tab Mix Plus, Tab Ultilities, and Multi Tab Handler(a very useful addon)...
A small feature but break that much, so I think we no need it, Chrome is a multi-process browser, it suits with animation, but Firefox not.

Comment 9

[Peter Henkel [:Terepin]]

Oh yeah, and while we are at it, we can revert completely to XP look, just for the sake of being different. Long live the democracy (even though running by idiots).

Comment 10

[Justin Dolske [:Dolske]]

After talking this through with Frank, we concur that the right thing to do here is to backout on Beta and Aurora. Similarly, Frank and I feel it also makes sense to just backout on Nightly as well (mostly just for ease of tracking and additional post-backout testing, but it's otherwise less-urgent to backout from Nightly).

Drivers make the final call, but there are issues as I see them:

* Steady increase in reported regressions (almost 30 dep-bugs right now).

* After landing we found that some of the regressions were from fundamental design flaws in the patch (ie, some of the OS interactions relied on dragndrop), and these issues are difficult or impossible to work around.

* While some of the regressions are fairly minor ("edge cases"), appearances can be deceiving. Some users are severely impacted by breakage due to their specific usage patterns, while others might see no problems. EG, our own Boriss Zbarsky is just being tortured by bug 675860. Tab drag/detach is very much a primordial piece of primary UI, and I regret not treating these regressions more seriously because of that.

[It's worth nothing that the first 2 points aboove developed slowly over time. If this had all happened within days of landing, we'd have backed it out immediately. Classic case of boiling a frog by heating the water slowly.]

Addon compatability is actually *not* a motivating factor here. The long term plan, backout or not, is to have this feature in Firefox (albeit implemented differently for the reasons above). As always we'll try to avoid gratuitous breakage, but in the end addons will have to adapt to any unavoidable changes. [I'd be interested in specific issues from addon authors regarding the current implementation, perhaps there are things we can do to make it an easier transition.]

Addon compatability is a mixed bag for a backout anyway... Some addons may start working again, but if other addons have already been changed to work with Aurora/Beta they could be broken. I don't know the scale of either case.

Comment 11

[Justin Dolske [:Dolske]]

Comment on attachment 564066 [details] [diff] [review]
backout patch (beta)

(Flagging for Aurora two, even if it might need a slightly different patch)

Comment 12

[Alex Limi (:limi) — Firefox UX Team]

Commenting on the issue at hand; whether we should keep this feature and ship it to our users once 8 goes live — or back all of it out, making the time frame for this feature making it at least Firefox 10, possibly 11.

I think this is a case of “edge case engineering” vs. user delight again. Just because we found some problems that affects a small percentage of our users, we're not going to ship it. I really dislike that approach, and I'd go as far as to say it's something we need to get away from.

We had a similar discussion around making add-ons compatible by default — because there was a potential to get a small percentage of our users in trouble (or even crashing!), we wouldn't ship it to the 400 million other people who wouldn't have any problems. Ultimately, some possible mitigation strategies were found for the worst cases, but we're still going to ship something that might crash thousands of users because of the long tail — because it makes the experience so much better for the 400 million others.

The tab dragging triggering in the wrong way in certain cases is ultimately a user experience issue. The code doesn't cause slowdowns or security risks.

The edge cases aren't destructive:
You don't lose content in the tab if you accidentally drag it off the tab strip, you just get a new window. Which can be annoying — but it's only triggered if the plugin container is positioned in a particular way, and if you happen to be dragging the tab when it's positioned like that.

As the UX team, we're comfortable shipping it like that, since the upside is so much bigger than the chances of people hitting these edge cases.

FWIW, Asa is in favor of shipping it too, and points out that this has been live on Beta (with a few million users) without major problems for quite a while already.

Comment 13

[Jorge Villalobos [:jorgev] (he/him)]

In the topic of add-on compatibility, I'd like to point out that this change broke tabbed browsing add-ons that amount to hundreds of thousands if not millions of users. Some of these add-ons are considered must-have, and it could lead to many users not upgrading or taking their problems to SUMO. I know I'm in that boat with Tree Style Tabs.

Comment 14

[YUKI "Piro" Hiroshi]

As an add-on author, I hope that the back-out patch become landed and the cool animation effect come back with high compatibility with native drag and drop behaviors on platforms.

I totally agree that rich visual effects are very important for user experience. Actually I love it and I'm willing to do hard work to update my addons for the new feature. I also think that it is a duty of add-on authors. I don't want to drag those efforts (for better UX) down, for end users.

By same reason, just one (but fatal) problem is the compatibility with standard behavior on the platform. I dislike too selfish UIs like old RealPlayer and so on. Even if the application provides cool UIs, it is fatal that it doesn't work with other applications via standard protocol on the platform - click, drag and drop, etc. It is just one reason that I cannot agree the patch from the bug 455694.

Comment 15

[[:Cww]]

Here's 24-hours of input of issues likely related to this.

http://input.mozilla.com/en-US/opinion/2441069
http://input.mozilla.com/en-US/opinion/2441068
http://input.mozilla.com/en-US/opinion/2440975
http://input.mozilla.com/en-US/opinion/2440732
http://input.mozilla.com/en-US/opinion/2440700
http://input.mozilla.com/en-US/opinion/2440704
http://input.mozilla.com/en-US/opinion/2440563
http://input.mozilla.com/en-US/opinion/2440203
http://input.mozilla.com/en-US/opinion/2439993
http://input.mozilla.com/en-US/opinion/2439454
http://input.mozilla.com/en-US/opinion/2439450
http://input.mozilla.com/en-US/opinion/2439439
http://input.mozilla.com/en-US/opinion/2439405
http://input.mozilla.com/en-US/opinion/2439259
http://input.mozilla.com/en-US/opinion/2439122
http://input.mozilla.com/en-US/opinion/2439028

I also think it's a little disingenuous to compare this to add-ons compatible by default.  Add-on compatibility is something thousands of users complain about with each release -- we don't get any complaining about our tab reordering.

I'm not going to argue whether we should pull this; it's not my decision. Just don't use "we don't have negative input" as a reason, please.

Comment 16

[Alex Limi (:limi) — Firefox UX Team]

I left out the word "significant" — as in the level we got for showing URL preview in URL bar, for example.

With 400 million users, you'll always have negative input — as you are acutely aware of. :)

Comment 17

[ithinc]

(In reply to Justin Dolske [:Dolske] from comment #10)
> The long
> term plan, backout or not, is to have this feature in Firefox (albeit
> implemented differently for the reasons above). As always we'll try to avoid
> gratuitous breakage, but in the end addons will have to adapt to any
> unavoidable changes.

I, as an add-on author, do welcome the new feature, but just dislike the implementation from bug 455694, which leads to many regressions and avoidable extra work for add-on authors.

> [I'd be interested in specific issues from addon
> authors regarding the current implementation, perhaps there are things we
> can do to make it an easier transition.]

Besides the DND API problem, it would be better for the implementation to consider more on vertical or multi-row tab bar mode. If we have a _dragDistX saved, save a _dragDistY also, etc. Firefox doesn't need to implement these features, but please provide the API convenience.

Comment 18

[dindog]

I still hope that when animation Drag & Drop tabs landed, it could provide an option to diabled it like other animation such as closing tab, tab candy... because of two obvious reasons:

1. I know quite many users complain those animations will slow down firefox response with older computer.

2. Whenever there is confliction with tabs' related add-ons, users could just disabled an option, not go back to previous compatitable versions of Firefox when the add-on is a "MUST HAVE" to them. Especially for those less maintained add-on

Comment 19

[Dão Gottwald [:dao]]

(In reply to Justin Dolske [:Dolske] from comment #10)
> Addon compatability is actually *not* a motivating factor here. The long
> term plan, backout or not, is to have this feature in Firefox (albeit
> implemented differently for the reasons above). As always we'll try to avoid
> gratuitous breakage, but in the end addons will have to adapt to any
> unavoidable changes.

This is missing the point. The premise in this bug is that add-on authors want to support this feature, but we put obstacles in their way.

> [I'd be interested in specific issues from addon
> authors regarding the current implementation, perhaps there are things we
> can do to make it an easier transition.]

First and foremost we need to use the drag and drop API.

Comment 20

[Asa Dotzler [:asa]]

(In reply to Jorge Villalobos [:jorgev] from comment #13)
> In the topic of add-on compatibility, I'd like to point out that this change
> broke tabbed browsing add-ons that amount to hundreds of thousands if not
> millions of users. Some of these add-ons are considered must-have, and it
> could lead to many users not upgrading or taking their problems to SUMO. I
> know I'm in that boat with Tree Style Tabs.

Jorge, besides Tree Style Tabs which add-ons explicitly did this break?

Comment 21

[Alex Limi (:limi) — Firefox UX Team]

Also, I realized that it might not be clear from the context (since he didn't mention it :), SHIMODA Hiroshi is the author of Tree Style Tabs. So I'm confident that particular extension will be updated.

Comment 22

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Asa Dotzler [:asa] from comment #20)
> (In reply to Jorge Villalobos [:jorgev] from comment #13)
> > In the topic of add-on compatibility, I'd like to point out that this change
> > broke tabbed browsing add-ons that amount to hundreds of thousands if not
> > millions of users. Some of these add-ons are considered must-have, and it
> > could lead to many users not upgrading or taking their problems to SUMO. I
> > know I'm in that boat with Tree Style Tabs.
> 
> Jorge, besides Tree Style Tabs which add-ons explicitly did this break?

There are a couple others mentioned in comment #2, and others in comment #8 (which I haven't confirmed). I'll come up with a better list tomorrow, but it's difficult to tell exactly which break without actually testing.

Comment 23

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Alex Limi (:limi) — Firefox UX Team from comment #21)
> Also, I realized that it might not be clear from the context (since he
> didn't mention it :), SHIMODA Hiroshi is the author of Tree Style Tabs. So
> I'm confident that particular extension will be updated.

You might want to read comment #2 not only for the necessary context, but also a statement claiming these extensions won't be updated as long as this bug is present. While I believe some breakage is much better than no support at all, maybe Piro (Shimoda Hiroshi) feels differently.

Comment 24

[YUKI "Piro" Hiroshi]

> You might want to read comment #2 not only for the necessary context, but
> also a statement claiming these extensions won't be updated as long as this
> bug is present.

I wrote two reasons why I won't update my extensions, and now I think the first one is more important than the second.

> 1) If the bug 674925 becomes fixed after I updated them for current Firefox 8
> (based on plain mouse events), they become obsolete soon. Then I have to update
> them again for the new Firefox 8. It is too painful.

If Mozilla project completely decides that this bug and regressions are completely WONTFIX in future (ex. Fx7=drag events, Fx8=mouse events, Fx9=mouse events, Fx10=mouse events, ...), I'll swallow the sad thing. I have less motivation to update my addons for plain mouse events, but it is better than keeping up to update them for changing implementations in future releases (ex. Fx7=drag events, Fx8=mouse events, Fx9=drag events, ...).

Comment 25

[ithinc]

(In reply to Alex Limi (:limi) — Firefox UX Team from comment #21)
> Also, I realized that it might not be clear from the context (since he
> didn't mention it :), SHIMODA Hiroshi is the author of Tree Style Tabs. So
> I'm confident that particular extension will be updated.

As the many other regressions of Firefox itself, I'm sure the extension (and many others) won't get updated before bug 674925 on this issue.

Comment 26

[Jorge Villalobos [:jorgev] (he/him)]

Here are some extensions I tested and found to break with this change:

Tab Mix Plus - 1.6M ADU - DND completely broken
Tree Style Tabs - 94K ADU * - DND completely broken
Multiple Tab Handler - 89K ADU * - DND partially broken
TabGroups Manager - 27K ADU - DND partially broken

* It's worth noting that Piro's add-ons are mostly distributed outside of AMO, so these numbers are not accurate.
@ Piro do you keep any usage stats of the add-ons downloaded from your site?

Comment 27

[Frank Yan (:fryn)]

Comment on attachment 564066 [details] [diff] [review]
backout patch (beta)

I applied it, and aside from a few minor merge conflicts, it checks out.
I'll attach an unbitrotted version.

Comment 28

[Frank Yan (:fryn)]

Attachment: backout patch (central)

Comment 29

[Dão Gottwald [:dao]]

My patch is for mozilla-beta, there's no bitrot.

Comment 30

[solcroft]

(In reply to Dão Gottwald [:dao] from comment #29)
> My patch is for mozilla-beta, there's no bitrot.

Quick question: does that mean that this patch is not planned for merge with Nightly?

Comment 31

[Dão Gottwald [:dao]]

Attachment: backout patch (aurora)

Comment 32

[Dão Gottwald [:dao]]

Christian, what's the decision here?

Comment 33

[Asa Dotzler [:asa]]

(In reply to Dão Gottwald [:dao] from comment #32)
> Christian, what's the decision here?

A decision hasn't been made. From yesterday's triage meeting, I understand we're expecting one early next week.

Comment 34

[LegNeato]

---------------------------------[ Triage Comment ]---------------------------------

We need to track this final decision. I am in the process of writing it up and getting sign-off from assorted parties ASAP.

Comment 35

[LegNeato]

Please land the backouts ASAP. This is NOT the official direction yet. Landing the backout will give us testing both ways so whatever we decide to do will be tested.

Comment 36

[LegNeato]

http://hg.mozilla.org/releases/mozilla-aurora/rev/5bf4059fa0e0
http://hg.mozilla.org/releases/mozilla-beta/rev/1e35c1015509

I did not back out of mozilla-central so this bug stays open.

Comment 37

[silverwav]

(In reply to SHIMODA Hiroshi from comment #2)
> Actually, I decided that I don't update some my addons for Firefox 8 until
> the bug 674925 (or fatal regressions caused by the bug 455694) become
> resolved. For example:
> 
>  * Tree Style Tab https://addons.mozilla.org/firefox/addon/tree-style-tab/
>  * Multiple Tab Handler
> https://addons.mozilla.org/firefox/addon/multiple-tab-handler/
>  * Fox Splitter https://addons.mozilla.org/firefox/addon/fox-splitter/
> 
> There are two reasons.
> 
> 1) If the bug 674925 becomes fixed after I updated them for current Firefox
> 8 (based on plain mouse events), they become obsolete soon. Then I have to
> update them again for the new Firefox 8. It is too painful.
> 
> 2) Even if I update them for the current Firefox 8, many regressions from
> the bug 455694 are still there. I'm worrying that I receive annoying bug
> reports from those regressions. Yes, of course I can "fix" them on my
> addons, but it is a bad way, because such ad-hock patches will break
> compatibility with other addons.

SHIMODA Hiroshi
----------------
Ahhh! Please post a note on AMO on your "Tree Style Tab" page noting this issue and that you are holding out for this to be Backed out please.

I have looked around for a fix for this issue (cant move tabs between groups) for a while.
I have even tried other extensions to see if I could work around the issue to no avail.


Firefox Team
-------------
I cant use Firefox without an extension providing vertical tabs.
I am sure there are a large amount of Firefox users who feel the same.
If this breaking feature is not backed out we will have a huge backlash.

Comment 38

[James Munroe]

(In reply to silverwav from comment #37)
> (In reply to SHIMODA Hiroshi from comment #2)
> > Actually, I decided that I don't update some my addons for Firefox 8 until
> > the bug 674925 (or fatal regressions caused by the bug 455694) become
> > resolved. For example:
> > 
> >  * Tree Style Tab https://addons.mozilla.org/firefox/addon/tree-style-tab/
> >  * Multiple Tab Handler
> > https://addons.mozilla.org/firefox/addon/multiple-tab-handler/
> >  * Fox Splitter https://addons.mozilla.org/firefox/addon/fox-splitter/
> > 
> > There are two reasons.
> > 
> > 1) If the bug 674925 becomes fixed after I updated them for current Firefox
> > 8 (based on plain mouse events), they become obsolete soon. Then I have to
> > update them again for the new Firefox 8. It is too painful.
> > 
> > 2) Even if I update them for the current Firefox 8, many regressions from
> > the bug 455694 are still there. I'm worrying that I receive annoying bug
> > reports from those regressions. Yes, of course I can "fix" them on my
> > addons, but it is a bad way, because such ad-hock patches will break
> > compatibility with other addons.
> 
> SHIMODA Hiroshi
> ----------------
> Ahhh! Please post a note on AMO on your "Tree Style Tab" page noting this
> issue and that you are holding out for this to be Backed out please.
> 
> I have looked around for a fix for this issue (cant move tabs between
> groups) for a while.
> I have even tried other extensions to see if I could work around the issue
> to no avail.
> 
> 
> Firefox Team
> -------------
> I cant use Firefox without an extension providing vertical tabs.
> I am sure there are a large amount of Firefox users who feel the same.
> If this breaking feature is not backed out we will have a huge backlash.

Try reading above, they backed it out to do more testing and move on from there. You have plenty of time to make sure they support your need in Firefox, but no need to keep on about things that have been done. To reiterate, the patches have been backed out, the world is not ending, you will survive.

Comment 39

[Dão Gottwald [:dao]]

Frank landed this on central:
http://hg.mozilla.org/mozilla-central/rev/4bb7c983d589

Comment 40

[AndreiD[QA]]

Since the patches to back out the Tab animation feature landed on all branches, I tested to see if this bug is fixed:
Verified fixed on Windows 7:
Mozilla/5.0 (Windows NT 6.1; rv:9.0a2) Gecko/20111027 Firefox/9.0a2 (Aurora)
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0 (Beta5)
Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111027 Firefox/10.0a1 (Nightly)
Verified fixed on Linux:
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0 (Beta5)
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111027 Firefox/10.0a1 (Nightly)
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111027 Firefox/9.0a2 (Aurora)
Verified fixed on Mac OS 10.7:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:8.0) Gecko/20100101 Firefox/8.0 (Beta5)
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0a2) Gecko/20111027 Firefox/9.0a2 (Aurora)
 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a1) Gecko/20111027 Firefox/10.0a1 (Nightly)

*As of these patches, tab management in Firefox 8 will be similar to the one available for Firefox 7. 
Setting this bug as verified and adding the tracking keyword [qa!]

Comment 41

[alessino]

Are there any plans to bring this feature back?

Comment 42

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to alessino from comment #41)
> Are there any plans to bring this feature back?

Yes, see https://bugzilla.mozilla.org/show_bug.cgi?id=674925