519128 - Separate message window closes when other message is Junked

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Robert Roessler]

When viewing [POP] email in a separate message window, that window often closes when new mail arrives.

I eventually noticed that this ONLY occurred when new mail arrived that was classified as spam, and never happened when NON-spam email arrived... it was harder to see this detail since the vast majority of email (at my 5-minute checking granularity) arrivals contain at least 1 spam message. ;)

Next, I noticed that MANUALLY declaring a particular message in the Inbox header listing to be Junk would also result in the closing of a separate message window.

Note that have the "move Junk to Junk folder" option SET, which will affect both the automatically and manually categorized junk.

So, while Bug 492699 and its comments are likely relevant, it is not obvious that there is only one bug here... and this is against SeaMonkey specifically.

I estimate the timeframe for this regression as 3-4 months ago on the SM2 trunk builds.

Comment 1

[WADA:World Anti-bad-Duping Agency]

Does problem occur with Sm 1.x? Sm 2.x only problem?
Do you enable auto-compact with mail.purge.ask=false? (call "silent auto-compact") 

If SM 2 only and "silent auto-compact", it's possibly same phenomenon to Bug 518920 Comment #4 on Tb 3 nightly.

Comment 2

[Robert Roessler]

I expect SM 1.x still works fine in this regard - it always did when I used it, and it has mostly been getting security fixes only for some time now, right?

For me, as it says in comment #0, this has been only in SM2 - trunk builds when it started (3-4 months ago?) and was still running Vista x64, and now in the current 2.0pre nightly builds on Win7 x64.

No, I don't use "silent auto-compact" - the mail.purge.ask is still defaulted to true.

Comment 3

[WADA:World Anti-bad-Duping Agency]

"Close of standalone message window" can be observed by "Rebuild-index" too with Shredder (checked with 9/30 build).
1. Open a mail in standalone window. Open Error Console.
2. Rebuild-Index => Thread pane is cleared (known phenomenon).
   I don't know whether it happens with Sm 2 or not.  
3. Standalone window is closed. 
4. Close Tb window -> next exception. 
> Error: uncaught exception: [Exception... "Could not convert JavaScript argument arg 0 [nsIObserverService.removeObserver]"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"
> location: "JS frame :: chrome://messenger/content/search.xml ::  :: line 293"  data: no]

If internal rebuild-index is invoked due to "wrong msf data" or "outdated msf condition", "message window close" can happen.
Junk move generates such condition on Inbox? Is there any error in Error Console?

Comment 4

[Robert Kaiser]

I can't tell if we want this before I know if this is intended behavior or not (and it might very well be, I have no clue)

Comment 5

[Robert Roessler]

This has NEVER worked like this in all the years I have used Mozilla (now SeaMonkey), or in the old Netscape Communicator, for that matter.

Why would we possibly want this ugly behavior?

Happily reading a message in a separate window, and then a random spam comes in, and suddenly your message disappears?  It seems unlikely that this could possibly be desirable behavior... and it did not do this until some unfortunate change some months ago.

Comment 6

[Ian Neal]

Could you just confirm whether this is happening on SM 2.1pre or SM2.0x as you do mention SM trunk in comment#0?

Comment 7

[Robert Roessler]

I mentioned the SM 2 trunk builds because that is what I was running (and first saw this regression in) until the betas appeared - which are now the "SM 2.0pre" builds, where the problem persists.

To try this on the current trunk ("SM 2.1a1pre") builds is more difficult... one of the problems being that there ARE no ["comm-central-trunk"] builds for Windows since October 1st.

I can set up an environment to test this on the last available trunk build - film at 11. ;)

Comment 8

[Robert Roessler]

This is indeed still present in both 2.0pre and 2.1a1pre.

More details: to observe the message window closing regression, you either need to manually mark some message still in the Inbox as being Junk (triggering the move to the Junk folder), OR a new batch of mail arrives in your Inbox, with at least one message being classified as Junk by the automated learning mechanism (also triggering a move from the Inbox to the Junk folder).

The moving from the Inbox to the Junk folder seems to be key here - for the subset of [Junk] mails that are caught by my manually specified filters, the message window-closing behavior does not appear...

I think the key difference is that these mails never make it as far as the Inbox, and are routed directly into the Junk folder, whereas the "normal" automatically detected Junk does first get placed into the Inbox prior to the Junk classification system taking a look at it.

Comment 9

[[e-mail]]

Bug 486954 removed gCurrentMessageIsDeleted, which seems to be relevant.

Comment 10

[Magnus Melin [:mkmelin]]

FWIW, i don't see this with thunderbird, but it's easy to repro with seamonkey. The folderdisplay refactoring in tb might have taken care of it.

Comment 11

[Karsten Düsterloh]

(In reply to comment #4)
> I can't tell if we want this before I know if this is intended behavior or not

No, it's not. The window should only close once the folder is empty (eg because all mails are deleted or marked as junk).

(In reply to comment #9)
> Bug 486954 removed gCurrentMessageIsDeleted, which seems to be relevant.

I don't think so.

"Junking" the mail isn't relevant for the actual bug either:
- select a message in the thread pane and
- open it in the standalone window
- now select a different message in the thread pane and
- delete it
=> the standalone window closes

The problem seems to be that in HandleDeleteOrMoveMsgCompleted the value of gNextMessageViewIndexAfterDelete is still -2 after the removal of the current (junk) message. AFAIUI, the backend should have called updateNextMessageAfterDelete before, but that never happens...

Comment 12

[Robert Kaiser]

From what Karsten says, this is wanted in any case, then - I wonder if it might even be blocking... Karsten?

Comment 13

[Karsten Düsterloh]

Attachment: only close if current

Okay, Neil is right; as always. ;-)

Without gCurrentMessageIsDeleted, we always fall down into HandleDeleteOrMoveMsgCompleted, even if the removed message isn't the one we're showing in the standalone window. If we _are_ deleting/junking the currently shown message, then closing the standalone window should depend upon messages left/gNextMessageViewIndexAfterDelete...

To cut a long explanation short: this patch just undos Magnus' suite part of bug   486954 attachment 372923 [details] [diff] [review].

Comment 14

[Karsten Düsterloh]

Pushed as <http://hg.mozilla.org/comm-central/rev/0d3261f63a0e>.

Comment 15

[Karsten Düsterloh]

BTW: this incidentally fixes the case where you lost the threadpane selection after junking multiple message at once.

Comment 16

[Robert Roessler]

Verified fixed in the October 13 nightly build of SM 2.0pre - thanks!