671486 - Merge closing window's pinned tabs into next remaining window

Status: RESOLVED WONTFIX

(Firefox :: Tabbed Browser, defect)

Description

[Frank Yan (:fryn)]

Attachment: patch

In the Firefox add-on I made called App Tabs almost two years ago, I found that the loss of the app tabs when a window closed was frustrating, and it's a problem we are still frequently seeing today, so in that add-on, I merged the app tabs from the closing window in a remaining window. IIRC, Felipe and Dão concurred that this is a decent idea if global app tabs cannot be fully and cleanly executed in the near future. In a way, it enforces the idea that app tabs don't and shouldn't really belong to any window.

I made a quick patch for this by adapting my code from the add-on, in case we want this. It also plays nicely with sessionstore AFAICT.

I pushed it to the UX branch, so you can play it tomorrow. You can be frank with me about what you think. ;)

Comment 1

[Asa Dotzler [:asa]]

Frank, I think this would go a long way to fixing the user perceptions around "losing" app tabs. Any chance you could get a review on this and get it into m-c in the next week? (in time for 8)

Comment 2

[Asa Dotzler [:asa]]

margaret, is this something you could tackle while fryn's out?

Comment 3

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

I think this is a great workaround until properly global app tabs land.

Comment 4

[Paul [pwd]]

I'm in compete disagreement with that. Every minute dedicated to this workaround would be better spent working on globalising app tabs so users can experience them as intended. This bug just sweeps the problem under the carpet.

Comment 5

[Asa Dotzler [:asa]]

Paul, the Firefox UX lead, the Firefox Product Manager (myself) and the developer working on this feature are in agreement that this is a fine short-term solution. I appreciate your attempt at project management here, but unless you're the one offering the resources to build the long-term solution, you're not really in a position to say how those resources are best spent.

Comment 6

[:Margaret Leibovic]

(In reply to Asa Dotzler [:asa] from comment #2)
> margaret, is this something you could tackle while fryn's out?

Sorry, I was also out! I think it's too late for this to make it in for Firefox 8. I have a lot I'm working on right now, but I can try to help Frank with this if he's still overloaded with other work.

Comment 7

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Asa Dotzler [:asa] from comment #5)
> the developer working on this feature are in agreement that this is a fine
> short-term solution.

I'm pretty sure I'm the developer working on the feature and I don't really like the idea of merging. But I agree that it's probably better than nothing and if we put it in for 8 then cool. Hopefully I'll finish bug 587873 for 9 and the whole thing will be moot.

Comment 8

[Asa Dotzler [:asa]]

If there's any chance we can get this reviewed and landed for 8 (tomorrow morning?) or potentially really early in Aurora, it'd be a nice stopgap before we have the full fix in for 9. If a review in the next few days can't happen or we can't make the case that it's safe enough to take this "pseudo-dataloss" fix into Aurora, then I guess we wait on 9.

Comment 9

[Frank Yan (:fryn)]

To be frank, I would rather not land a "detour" like this if Paul is serious about global app tabs. Even though I've been skeptical of our planned app tab model from the beginning, I don't want to make it even more confusing.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

A detour isn't ideal, but it might still be better than having a whole 6 weeks where we are just closing the app tabs entirely.  So in order of preference:

1) global app tabs
2) grouping app tabs into the next window
3) doing nothing

I agree with Asa that we should try to avoid the dataloss, even with our only option is a quick detour.

Comment 11

[ithinc]

Comment on attachment 545827 [details] [diff] [review]
patch

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

Generally, this is a very neat solution. No risk as I can see.

::: browser/components/sessionstore/src/nsSessionStore.js
@@ +932,5 @@
> +      if ((winData.tabs.length > 1 ||
> +           (winData.tabs.length == 1 && this._shouldSaveTabState(winData.tabs[0]))) &&
> +      // and at least one non-app tab, since app tabs will be merged to the
> +      // next remaining window
> +          tabbrowser._numPinnedTabs < tabbrowser.tabs.length) {

Maybe this is not necessary? This should be called on "domwindowclosed" when the app tabs have been removed.

Comment 12

[Siddhartha Dugar [:sdrocking]]

This feels like a simpler and more intuitive solution than having global app tabs. Having the same app tab on different windows would require extra effort to keep them in the same state and could look confusing and redundant. This fix instead would be simple and intuitive for everyone.

Comment 13

[Frank Yan (:fryn)]

(In reply to ithinc from comment #11)
> ::: browser/components/sessionstore/src/nsSessionStore.js
> @@ +932,5 @@
> > +      if ((winData.tabs.length > 1 ||
> > +           (winData.tabs.length == 1 && this._shouldSaveTabState(winData.tabs[0]))) &&
> > +      // and at least one non-app tab, since app tabs will be merged to the
> > +      // next remaining window
> > +          tabbrowser._numPinnedTabs < tabbrowser.tabs.length) {
> 
> Maybe this is not necessary? This should be called on "domwindowclosed" when
> the app tabs have been removed.

I'll look into it.

(In reply to sdrocking from comment #12)
> This feels like a simpler and more intuitive solution than having global app
> tabs. Having the same app tab on different windows would require extra
> effort to keep them in the same state and could look confusing and
> redundant. This fix instead would be simple and intuitive for everyone.

You provide no support for your claims and repeat yourself. Please try to add something substantive when commenting.

Comment 14

[Siddhartha Dugar [:sdrocking]]

Over 1 month passed but there is no update on this. Please fix this for Firefox 10.

Comment 15

[Frank Yan (:fryn)]

If we stick to promoting the mental model that pinned tabs are simply tabs pinned to a window and not persistent forever and ever -- and I think that that is indeed our path forward -- then it does not make sense to fix this implement this.