Closed
Bug 588217
Opened 14 years ago
Closed 9 years ago
Integrate Tab Groups data into Session Restore proper
Categories
(Firefox :: Session Restore, defect, P2)
Firefox
Session Restore
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: zpao, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Right now Tab Candy is using Session Restore to stash data. However this approach is falling short in a few ways:
Undo Close Tab knows nothing about tab sets. So recently closed tabs are shown from all tab sets. This should be local to the current tab set (and stored independently). If we're going to handle this inside session restore, then we need to actually acknowledge that Tab Candy exists & is using session restore for data.
If tab sets become window-independent, then we need to restructure how session data is stored.
Current:
> {
> "windows":[
> {
> "tabs":[
> { ... },
> { ... }
> ],
> "_closedTabs":[...],
> }
> ],
> "_closedWindows":[...]
> }
Proposed (haven't necessarily covered all details)
> {
> "tabsets":[
> {
> "id":100100101010,
> "title": "foo",
> "tabs":[
> { ... },
> { ... }
> ],
> "_closedTabs":[...],
> },
> ]
> "windows":[
> {
> "tabset":100100101010,
> },
> { ... }
> ]
> }
This would break consumers, hard. If it's going to happen, it should happen now.
We'd need new APIs. We'd need to get rid of some old APIs
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 1•14 years ago
|
||
If tab sets become window independent, we could save them as window objects, with a type=set property. Not only would that not fully break add-on consumers (worst case they report more windows. but no dataloss!), it would also keep the data backwards compatible*.
(* If it even is, anymore. I'm not sure.)
Comment 2•14 years ago
|
||
By the way, this bug is related to bug 578512 (manage all windows).
Reporter | ||
Comment 3•14 years ago
|
||
I'll add another reason why this would be good...
This makes it much easier (in association with bug 586068 and maybe something else that we'd need to file) to prioritize loading a specific group. We would be able to only load a tabset once it's actually being used. I know it's been pointed out by John Bird that loading all the tabs (a) takes long time and (b) uses a lot of memory. This gets worse with more tabs being "hidden" behind tab sets. Should definitely be a new bug, but could happen later (since it wouldn't be stringy or really impact behavior too much)...
Comment 4•14 years ago
|
||
Ian: we probably need to put a bug on file about the APIs required for this and similar add-on requests to gain awareness of tab candy constructs like "groups" and which tabs are visible in which groups. I've seen several requests for this as a cleaner way of dealing with the hidden/visible aspect of things.
Updated•14 years ago
|
Comment 5•14 years ago
|
||
(In reply to comment #4)
> Ian: we probably need to put a bug on file about the APIs required for this and
> similar add-on requests to gain awareness of tab candy constructs like "groups"
> and which tabs are visible in which groups. I've seen several requests for this
> as a cleaner way of dealing with the hidden/visible aspect of things.
Done, bug 588899.
Comment 6•14 years ago
|
||
--> dietrich, since apparently he volunteered to do the session restore piece of this.
Assignee: nobody → dietrich
Comment 7•14 years ago
|
||
What APIs will be changed as a part of this? This sounds offhand like it will touch extension-visible APIs and should block beta5, but I'd love to hear that it doesn't.
blocking2.0: ? → beta5+
Comment 8•14 years ago
|
||
(In reply to comment #1)
> If tab sets become window independent, we could save them as window objects,
> with a type=set property. Not only would that not fully break add-on consumers
> (worst case they report more windows. but no dataloss!), it would also keep the
> data backwards compatible*.
>
> (* If it even is, anymore. I'm not sure.)
Just double checking that this does not mean that each tab group will be represented by creating a physical window?
Comment 9•14 years ago
|
||
(In reply to comment #8)
> Just double checking that this does not mean that each tab group will be
> represented by creating a physical window?
Yes and no. I was describing a way to model groups in session restore's internal storage, such that the group data wouldn't get lost if a user's migration path looked like: 3.* -> 4.* -> 3.* -> 4.*.
A side-effect of this approach is that groups *would* appear as actual windows in 3.*. There are a lot of other problems with the approach. We can probably write-off that upgrade scenario and relnote it. If the group data is synced via Weave then we can call that a workaround.
Comment 10•14 years ago
|
||
The thought of 3 and 4 integration hadn't even crossed my mind. *shudders*.
+1 to the approach.
Comment 11•14 years ago
|
||
* at data collection time, collect group data and add to the object that is serialized and written to disk.
TODO:
* remove all the manual storage code in tab candy, outside of saving the group id on each tab (we can fix that later)
* update groups.jsm with an api for bulk-setting the group data when a session is restored
Comment 12•14 years ago
|
||
The approach I'm taking with the tabview part of this:
1. when a session is restored, we add all the group/tab data from the ss data via TabViewGroup.createGroup(). the first time tabview is loaded, the data is already there, ready to go.
2. remove the load/save code entirely from tabs.jsm/groups.jsm. the only thing that the tabview code will need to do is get/set the group id on each tab. it's currently doing that when loading/saving all a tab's data. instead i'll integrate it into the groupID getter/setter on each TabViewTab. wrt migration, i'll leave the loading apis in place for a while (until beta after next probably), so that the first time this code is run, we pull the data from the old location (per-window), but subsequent loadings willl use the pre-populated data from the session.
3. remove groups.jsm's dependency on windows altogether. it was only there because the data was stored per-window. now that it'll be re/stored by nsSessionStoreService, it's no longer necessary for groups.jsm to be window-aware at all.
Comment 13•14 years ago
|
||
Attachment #469557 -
Attachment is obsolete: true
Comment 14•14 years ago
|
||
Somehow I doubt this will make b5, moving to b6
blocking2.0: beta5+ → beta6+
Comment 15•14 years ago
|
||
(In reply to comment #14)
> Somehow I doubt this will make b5, moving to b6
Correctomundo, hard dependency on big tabview changes in bug 578512 which got pushed to b6.
Comment 16•14 years ago
|
||
Comment on attachment 469911 [details] [diff] [review]
wip2 - changes to ss for restoration
Dietrich, the patch looks good so far, and I agree with the plan you've outlined. Thanks!
Comment 17•14 years ago
|
||
Changing to blocking- and unassigning for now, since bug 578512 isn't going to be fixed for Fx4.
The only bug this blocks that we might want for Fx4 is bug 587874. However, it doesn't seem like a ship-blocker to me.
Assignee: dietrich → nobody
blocking2.0: beta6+ → -
Comment 18•14 years ago
|
||
Saw this in the Panorama b8 dependency tree. Questions:
1. Will/should this work be pursued before bug 578512, which is now out of the scope of fx4?
2. Alternatively, should we not expect further work on this bug until much later, in which case we should drop its blocking of bug 591704?
Summary: Integrate Tab Candy data into Session Restore proper → Integrate Tab Groups data into Session Restore proper
Comment 19•14 years ago
|
||
(In reply to comment #18)
> Saw this in the Panorama b8 dependency tree. Questions:
>
> 1. Will/should this work be pursued before bug 578512, which is now out of the
> scope of fx4?
> 2. Alternatively, should we not expect further work on this bug until much
> later, in which case we should drop its blocking of bug 591704?
I believe number 2; removing block.
No longer blocks: 591704
Comment 20•14 years ago
|
||
Unless people feel strongly about this, I'm punting to the future as it doesn't seem like we need this to ship.
Priority: -- → P2
Target Milestone: --- → Future
Updated•14 years ago
|
Comment 21•14 years ago
|
||
(In reply to comment #20)
> Unless people feel strongly about this, I'm punting to the future as it doesn't
> seem like we need this to ship.
Not sure why this has been punted. Without being able to save & restore the grouped tabs on next start, Tab Candy seems more work and just eye-candy than actually being useful.
All the sorting/naming of the groups seems to be in vain, when it needs to be redone again.
More users getting confused by the behavior > http://support.mozilla.com/en-US/questions/758762
Can we get this for firefox4 final?
Reporter | ||
Comment 22•14 years ago
|
||
(In reply to comment #21)
> Can we get this for firefox4 final?
At this point, no.
Reporter | ||
Updated•14 years ago
|
Reporter | ||
Comment 23•14 years ago
|
||
I should have said this before instead of going for the multi-step email spam. Sorry.
(In reply to comment #21)
> Not sure why this has been punted. Without being able to save & restore the
> grouped tabs on next start, Tab Candy seems more work and just eye-candy than
> actually being useful.
Grouped tabs are saved and restored on next start. While Panorama won't load the group data immediately at startup, the last visible group should be shown and group data (names, positions, etc) is available when Panorama loads.
> All the sorting/naming of the groups seems to be in vain, when it needs to be
> redone again.
> More users getting confused by the behavior >
> http://support.mozilla.com/en-US/questions/758762
The top answer there is wrong. That data is restored and AFAIK has been since before b6. I know it is for b7+.
Comment 24•14 years ago
|
||
>Unless people feel strongly about this, I'm punting to the future as it doesn't
>seem like we need this to ship.
We can't really do these three things together:
-punt on this
-punt on the home tab
-remove the save on close dialog box
the result of all three is that users are relying on a history menu command (or a preference they have to manually flip) in order to restore all of the data that they have created in tab candy (groups, names, positions, etc).
Possible solutions in order of difficulty:
1) add the restore session command to tab view (for when the user is wondering where all their stuff went)
2) automatically restore, but use bar-tab like loading for tab groups
3) automatically restore, use bar-tab like loading for tab groups, and introduce a distinction between groups that the user wants to permanently save or not (star icon instead of pencil, only way to name a group is to star it)
I think 3 is the best interface, but realistically we probably only have time for 1 or 2.
Comment 25•14 years ago
|
||
(In reply to comment #23)
> I should have said this before instead of going for the multi-step email spam.
> Sorry.
>
> (In reply to comment #21)
> > Not sure why this has been punted. Without being able to save & restore the
> > grouped tabs on next start, Tab Candy seems more work and just eye-candy than
> > actually being useful.
>
> Grouped tabs are saved and restored on next start. While Panorama won't load
> the group data immediately at startup, the last visible group should be shown
> and group data (names, positions, etc) is available when Panorama loads.
>
No, it does *not* . Well atleast in Ubuntu Linux, running 4.0~b8~hg20101128r58314+nobinonly-0ubuntu1~umd1~maverick.
Everytime I close using the Minefield > 'Quit' menu option.
And Start FF4 using a launcher where the command is "firefox-4.0" .
I have waited a while after starting FF4 too, but the groups are *never* available in Panorama. I have tried saving the groups alone, tried saving the groups with pages loaded in the tabs. I have even waited 5mins after starting FF4 waiting for Panorama to 'load' and not performing any other action, not once were the groups available the next time. Hence I mentioned that there seems no use in doing any action with the groups. It just isnt working here.
If this isnt the bug, should I open a separate bug regarding this problem?
Reporter | ||
Comment 26•14 years ago
|
||
(In reply to comment #25)
> If this isnt the bug, should I open a separate bug regarding this problem?
Please do. Make sure I'm CCed and we can discuss your issue there.
This bug is mostly just a change in who is in control of the data when we startup/quit. Instead of Panorama telling sessionstore to hold data, sessionstore should query Panorama for data or tell Panorama it has data when it's ready. While this does have an effect on user-facing features, it's not something that would change the game. It would just make many of the internals easier.
Comment 27•14 years ago
|
||
(In reply to comment #26)
> (In reply to comment #25)
> > If this isnt the bug, should I open a separate bug regarding this problem?
>
> Please do. Make sure I'm CCed and we can discuss your issue there.
>
Gah! it was a PEBKAC! not a bug. ;-)
With FF3 and earlier, I had set the Start-Up Preferences as "Show my Home Page".
And when I had migrated to FF4 it carried over the prefs, and hence the 'issue' of not storing tab groups.
When I changed it to "Show windows and tabs from last time" it works.. Sorry for the noise. :s
Comment 28•14 years ago
|
||
>and hence the 'issue'
yeah, that's not an "issue" but rather an issue. We can't ship with a default pref that eliminates user data.
Comment 29•14 years ago
|
||
(In reply to comment #28)
> >and hence the 'issue'
>
> yeah, that's not an "issue" but rather an issue. We can't ship with a default
> pref that eliminates user data.
Let's not give up. :-)
I filed Bug 618243 , maybe we can prevent this scenario.
(In reply to comment #24)
> Possible solutions in order of difficulty:
> 2) automatically restore, but use bar-tab like loading for tab groups
I tried bar-tab and it works awesome with TabGroups! Not loading the tabs/groups on startup saves a lot of on startup time.
Updated•14 years ago
|
Assignee: nobody → tim.taubert
Status: NEW → ASSIGNED
Target Milestone: Future → ---
Updated•14 years ago
|
Comment 32•13 years ago
|
||
bugspam
(Fx7 was branched, removing open bugs from Fx7 meta bug, we won't create new meta bugs for upcoming Fx versions)
No longer blocks: 660175
Updated•12 years ago
|
Assignee: ttaubert → nobody
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•