Closed Bug 671486 Opened 13 years ago Closed 12 years ago

Merge closing window's pinned tabs into next remaining window

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: fryn, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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. ;)
Blocks: 587400
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)
margaret, is this something you could tackle while fryn's out?
I think this is a great workaround until properly global app tabs land.
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.
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.
(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.
(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.
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.
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.
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 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.
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.
(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.
Over 1 month passed but there is no update on this. Please fix this for Firefox 10.
Summary: Merge closing window's app tabs into next remaining window → Merge closing window's pinned tabs into next remaining window
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: