296422 - Difficulty reverting to Classic 3 pane view

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Tracy Walker [:tracy]]

Seen on windows Thunderbird trunk build 2005-06-02-06-trunk

-Go to View | Layout
-Select Wide View then switch to Vertical View
-Finally, attempt to switch back to Classic View

tested results:  notice the preview pane becomes a bit wider. But the 3 pane
arrangement is still in Vertical View

expected results: 3 Pane view returns to Classic arrangement.

work-around:
switch back to Verticle View. Once again, switch to Classic View. 
The same can be said if trying to revert to Classic view while set in Wide View.
 Go back to Wide View, then switch to Classic a second time.

Comment 1

[Tracy Walker [:tracy]]

I saw this today on Mac Thunderbird trunk build but now on Windows.  I then
rechecked on Mac and could not reproduce it.  It has turned out to be randomly
reproducable. :-/  I will pay more attention as to discover a more reliable 
method to test it.

Comment 2

[Adam Guthrie]

*** Bug 309059 has been marked as a duplicate of this bug. ***

Comment 3

[Ian Hummel]

I've reproduced this with thunderbird 1.5 on linux and done some digging...

when switching layouts, we drop into UpdateMailPaneConfig in msgMail3PaneWindow.js.  When switching to/from wide view, we reparent threadpane-splitter and messagepanebox and update the flag msgPaneReRooted.

once this is done, we drop into this code:

  // re-rooting the message pane causes the docshell to get destroyed
  // and replaced with another one. As such, we need to re-set the window (and thus the
  // internal references nsMessenger keeps for the message pane docshell)
  // XXX: is it safe to call this multiple times? May need to add a setMessagePaneDocShell
  //      routine to nsIMessenger.
  if (aMsgWindowInitialized && msgPaneReRooted)
  {
    messenger.SetWindow(null, null);
    messenger.SetWindow(window, msgWindow);
    MsgReload();
  }

but, that call to MsgReload fails, raises an (uncaught) exception, we return from the function and we never update gCurrentPaneConfig, so the next time we switch layouts, we hit this:

 if (paneConfig == gCurrentPaneConfig)
    return;

and do nothing.

The call to MsgReload() fails because/only when no message is selected, which may be why Tracy Walker couldn't reliably reproduce it on mac.

Steps i've taken to reliably reproduce are:

1) start thunderbird.  switch to classic view.
2) exit thunderbird.
3) restart thunderbird... you should be in classic view now.
4) do not click on any messages
5) switch to wide view (notice the messagepane is now blank)
6) do not click on any messages
7) switch to class view and note that it won't work.

the comment in the code "// re-rooting the message pane causes the docshell to get destroyed" indicates that this also destroys or resets the nsIMsgDBView (gDBView)... 

i wish i had a patch, but that dbview stuff confused the hell out of me :)

Comment 4

[Tracy Walker [:tracy]]

another work-around: restart Tbird between changes.

nominating for TB2. Anyone experimenting with Pane View choices will find it difficult to get back to the default. 

Comment 5

[Scott MacGregor]

adding to the 2.0 bug work load bucket.

Thanks for filing this Tracy and thanks to Iann for the analysis, hopefully that will make it easier to fix.

Comment 6

[Scott MacGregor]

Attachment: the fix

This could be fixed in the front end (MsgReload) or in the back end. I chose the back end. Make nsMsgDBView::ReloadMsg return NS_OK if called when we don't have a selected message instead of throwing an error.

While I was debugging this problem, I noticed LoadMessageByUri, and OpenURL in commandglue.js were not used anymore. I was then able to remove gCurrentDisplayedMessage from msgMail3PaneWindow.js which was nice because we have a similarly named variable, gCurrentlyDisplayedMessage declared a few lines below that one :).

Comment 7

[Scott MacGregor]

fixed branch and trunk.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> While I was debugging this problem, I noticed LoadMessageByUri, and OpenURL in
> commandglue.js were not used anymore. I was then able to remove
> gCurrentDisplayedMessage from msgMail3PaneWindow.js which was nice because we
> have a similarly named variable, gCurrentlyDisplayedMessage declared a few
> lines below that one :).

From a LXR search, it would seem that the cleanup part could be ported to SM/MailNews:
http://lxr.mozilla.org/mozilla/search?string=OpenURL
http://lxr.mozilla.org/mozilla/search?string=LoadMessageByUri
http://lxr.mozilla.org/mozilla/search?string=gCurrentDisplayedMessage

Shall I prepare and submit such a patch in this bug !?