Open Bug 533908 Opened 11 years ago Updated 7 years ago

SeaMonkey Mail: tabs not restored

Categories

(SeaMonkey :: Session Restore, enhancement)

enhancement
Not set
normal

Tracking

(seamonkey2.1 wontfix)

ASSIGNED
Tracking Status
seamonkey2.1 --- wontfix

People

(Reporter: rvenkat77, Assigned: misak.bugzilla)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

16.90 KB, patch
misak.bugzilla
: feedback?
neil
mnyromyr
: feedback-
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4) Gecko/20091017 Lightning/1.0pre SeaMonkey/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4) Gecko/20091017 Lightning/1.0pre SeaMonkey/2.0

I see that SeaMonkey does not restore mail tabs when a session is closed and the tool is restarted. Browsing tabs are restored fine. Thunderbird3.0 restores mail tabs fine, and this capability appears to be missing in SeaMonkey2.0.

Reproducible: Always

Steps to Reproduce:
1. Open several mail tabs and browser tabs
2. Close session and reopen browser and mail
3. Browser tabs are reopened fine, mail tabs are not


Expected Results:  
All mail tabs including the lightning calendar should be reopened
That feature is still missing for mailnews, confirming as new enhancement request.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Component: General → Session Restore
Depends on: smtabmail
Ever confirmed: true
OS: Windows XP → All
QA Contact: general → session.restore
Hardware: x86 → All
Version: unspecified → Trunk
Depends on: 408338
Depends on: 555977
No longer depends on: 408338
No longer depends on: 555977
Duplicate of this bug: 555977
Flags: wanted-seamonkey2.1+
Depends on: 408338
Attached patch WIP 1Splinter Review
I did initial port/merge of bug 408338 to our suite sessionstore. What patch does:

Uses as much existing sessionstore code as possible.
Tries to preserve API compatibility with Thunderbird sessionstoreManager.js
Almost works, but as our tabmail seems not have persistTab function defined for mailnews tabs, it saves only calendar tab state.
Have some problems at end, sessionstore.json have empty mailnews state after SeaMonkey shutdown, seems i did too much code killing.
Requesting feedback from KaiRo, Neil, Mnyromyr and anyone else who interested.

Can someone more experienced in MailNews than me write persistTab for our tabs, i can copy/paste Thunderbird stuff, but it seems they go far to be easy thing to do ?
Assignee: nobody → misak.bugzilla
Status: NEW → ASSIGNED
Attachment #437007 - Flags: feedback?(neil)
Attachment #437007 - Flags: feedback?(mnyromyr)
Attachment #437007 - Flags: feedback?(kairo)
Flags: wanted-seamonkey2.1+
Attachment #437007 - Flags: feedback?(jh)
Attachment #437007 - Flags: feedback?(iann_bugzilla)
Comment on attachment 437007 [details] [diff] [review]
WIP 1

Sorry for letting this slip for so long. :-(

>+  /**
>+   * Get the current MailNews state.
>+   * @return a JSON string representing the session state.
>+   */
>+  AString getMailNewsState();

Given the origin of the sessionrestore code (a mere browser), it's not surprising to see the original IDL caring for browser windows only, hence adding a new function for the messenger may feel natural. 
But on second thought it doesn't make much sense, imo: 
- Do we really expect to save only one type of window data? That is, just browser, but not messenger (or whatever windows' states we might want to take of in future), or vice versa? I don't think that'd make any sense. Should we have even preferences for not saving certain stuff? I don't regard that as useful either.
- Currently, crashing "in" one type of window will kill all other windows anyway, we need to hope to have saved everything before anywhere.
- When we merge our tabbrowser implementations, windows are able to have different kind of tabs with save-worthy states.

We possibly should just have a method "getAppState" which would care for all kinds of save-worthy windows/tabs.
Do we care about IDL/API compatibility here? TB didn't either, actually, and we have even more special needs... 
In fact, TB's sessionstore feels like a hacky workaround indeed. We already have "fun" enough with two different tabbrowser implementations - we could of course use the TB approach "to get it in", but the cost of cleaning up that mess later will be rather high. :-(

Our sessionstore code lives under /common, so I'm rather reluctant about having messenger code there, but browser knowledge shouldn't probably live there either - maybe we can make the gory details "pluggable"? The actual persistency code for tabs is called via callbacks into the respective tabbrowser code already...

>+  _getCurrentMailNewsState: function sss_getCurrentMailNewsState() {
...
>+    // XXX we'd like to support other window types in future, but for now
>+    // only get the 3pane windows.

That's okay for now, since the standalone window will fold into the 3pane nicely in future anyway. 
(But see above why I don't think that a different functionality for messenger windows is needed.)

>+    while (enumerator.hasMoreElements()) {

As a side note: Please read <https://wiki.mozilla.org/index.php?title=SeaMonkey:MailNews:CodingStyle>.

>+      if (win && "complete" == win.document.readyState &&

I'd prefer a 'var == const' style of comparisons. ;-)

>+function getWindowStateForSessionPersistence()
>+{
>+  let tabmail = document.getElementById('tabmail');
>+  let tabsState = tabmail.persistTabs();
>+  return { type: "3pane", tabs: tabsState };

No whitespace before/after the curly braces, please.

In either case, I'm going to write up the persistTab/restoreTab methods which we will need in either case.

>+      <method name="persistTabs">
...
>+            // Explicitly specify a revision so we don't wish we had later.
>+            rev: 0,

This should be a constant somewhere.

>+            // If there is a non-null tab-state, then persisting succeeded and
>+            //  we should store it.  We store the tab's persisted state in its
>+            //  own distinct object rather than mixing things up in a dictionary
>+            //  to avoid bugs and because we may eventually let extensions store
>+            //  per-tab information in the persisted state.

Why did you leave out the tabMonitor code?

>+      <method name="restoreTabs">
...
>+            // The first tab is a folder tab

This is not true in SeaMonkey. The first messenger tab is not special for us.
Attachment #437007 - Flags: feedback?(mnyromyr) → feedback-
Attachment #437007 - Flags: feedback?(jh)
Attachment #437007 - Flags: feedback?(iann_bugzilla)
Comment on attachment 437007 [details] [diff] [review]
WIP 1

Oh, and while trying to use this patch to test my persistTab function, I found that it will never be called after startup - the timed save functionality is missing. It'll only work if a browser window is open, because the timer will be triggered from there.
(In reply to comment #5)
> Oh, and while trying to use this patch to test my persistTab function

I guess that function is needed for this patch to work correctly, where is your work on this function happening?

It would be so nice to see some progress in here...
(In reply to comment #4)
> (From update of attachment 437007 [details] [diff] [review])
> We possibly should just have a method "getAppState" which would care for all
> kinds of save-worthy windows/tabs.
Currently the behaviour for browser windows is that nothing gets restored until you next open a browser window. Of course the browser window is the default startup setting, which helps. I'm inclined to think that each type of restorable window should honour its own startup setting independently.

> Do we care about IDL/API compatibility here? TB didn't either
To try to persuade add-on developers to target us we have to make our API as close as the one they're already using as possible. So even if TB isn't compatible with Firefox it might be worth creating a frankinterface which combines the worst of both just because it makes it more portable.
Misak, still on your TODO?

(either way it won't land for 2.1)
Comment on attachment 437007 [details] [diff] [review]
WIP 1

I'm not really that active in SeaMonkey code any more, and there's no substantial build system change in here, so canceling feedback.
Still would be nice for the project to see this move forward, though.
Attachment #437007 - Flags: feedback?(kairo)
the last comment is quite telling. after mozilla officially dumped TB, poor SM is probably close to next on the block. if there are any SM developers left; i think many would thank any one of you if this problem were to be solved before you, too, left.

--
thank you,

johann
(In reply to johann from comment #10)
> the last comment is quite telling

It's just a personal change of focus. This happens.

> after mozilla officially dumped TB, poor SM is probably close to next on the block.

No. We do our own decisions.

But all this doesn't belong here, but into the group mozilla.dev.app.seamonkey or mozilla.support.seamonkey.
after my general observations, i pointed out that getting the mail-tab restoration working would be much appreciated. this request does seem to belong here.
You need to log in before you can comment on or make changes to this bug.