Merge closing window's pinned tabs into next remaining window

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
7 years ago
5 years ago

People

(Reporter: fryn, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Created attachment 545827 [details] [diff] [review]
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. ;)

Updated

7 years ago
Blocks: 587400

Comment 1

7 years ago
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

7 years ago
margaret, is this something you could tackle while fryn's out?
I think this is a great workaround until properly global app tabs land.

Comment 4

7 years ago
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

7 years ago
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

7 years ago
(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.

Comment 8

7 years ago
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.
(Reporter)

Comment 9

7 years ago
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 11

7 years ago
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.
(Reporter)

Comment 13

7 years ago
(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.

Updated

6 years ago
Summary: Merge closing window's app tabs into next remaining window → Merge closing window's pinned tabs into next remaining window
(Reporter)

Comment 15

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.