566704 - On mail deletion error 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgDBView.hdrForFirstSelectedMessage]" (Error: An error occurred updating the button_delete command)

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Igor Velkov]

On deleting mail message error console prints error message 2 times on every message:

Error: An error occurred updating the button_delete command: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgDBView.hdrForFirstSelectedMessage]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://messenger/content/mailWindowOverlay.js :: SelectedMessagesAreDeleted :: line 891"  data: no]
Source file: chrome://global/content/globalOverlay.js
Line: 86

reproduce:

open seamonkey, mailnews in safe-mode
nave at least 1 mail in some folder 
have 3 pane view (folders, mal list, mail body)
have error console opened
select mail in list
delete mail with toolbar "delete" button, or "del" keyboard button
hae error in error console

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

I cannot reproduce this (yet) but I think it's probably a race condition:

The error NS_ERROR_UNEXPECTED probably comes from nsMsgDBView::GetKeyForFirstSelectedMessage (called from nsMsgDBView::GetHdrForFirstSelectedMessage) and means that the selection is empty or invalid.

Our SelectedMessagesAreDeleted() code first checks for gDBView.numSelected and then for gDBView.hdrForFirstSelectedMessage.flags. I think in between those two checks the situation can internally, leading to the error message:

http://mxr.mozilla.org/comm-central/source/suite/mailnews/mailWindowOverlay.js#888

TB is probably not affected because they get the selected message once and check the flags of that:

http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.js#1028

We support gFolderDisplay.selectedMessage so one way to fix the bug would be to adapt TB's code for that particular function. Another would be to use try/catch. Mnyromyr, thoughts?

Comment 2

[Michael Lueck]

I seem to have been able to produce this on Ubuntu Linux running:
Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20101008 Firefox/4.0b7pre SeaMonkey/2.1b1

Working on another bug I was instructed to check the "Error Console (Tools/Web Development)" for errors. I opened the window, and two of these were right next to each other:

Error: An error occurred updating the cmd_delete command: [Exception...
"Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED)
[nsIMsgDBView.hdrForFirstSelectedMessage]"  nsresult: "0x8000ffff
(NS_ERROR_UNEXPECTED)"  location: "JS frame ::
chrome://messenger/content/mailWindowOverlay.js :: SelectedMessagesAreDeleted
:: line 892"  data: no]
Source File: chrome://global/content/globalOverlay.js
Line: 86

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

Unfortunately, since I got no reply to comment 1, I forgot about this bug. Looking at bug 588006 reminded me of this one. We should get rid of this error message (which is just a symptom of bug 588006).

I think try/catch is inferior to the TB approach, so I ported it.

Comment 4

[[e-mail]]

(In reply to comment #1)
> We support gFolderDisplay.selectedMessage so one way to fix the bug would be to
> adapt TB's code for that particular function.
That doesn't actually work, because our gFolderDisplay.selectedMessage simply relies on gDBView.numSelected anyway, something like this:
   gFolderDisplay.selectedMessage && gFolderDisplay.selectedMessage.flags
== gFolderDisplay.selectedCount && gDBView.hdrForFirstSelectedMessage.flags
== gDBView && gDBView.numSelected && gDBView.hdrForFirstSelectedMessage.flags

(In reply to comment #1)
> The error NS_ERROR_UNEXPECTED probably comes from
> nsMsgDBView::GetKeyForFirstSelectedMessage (called from
> nsMsgDBView::GetHdrForFirstSelectedMessage) and means that the selection is
> empty or invalid.
Perhaps gDBView.getIndicesForSelection({}).length gives a more accurate count.

Comment 5

[[e-mail]]

Actually it depends on what sort of count you want.

If you want to know how many messages will be affected by an operation, which depends on the preference mail.operate_on_msgs_in_collapsed_threads, then you need to use .numSelected, although that seems sometimes to return a nonzero value even though key/hdrForFirstSelectedMessage throws. By comparison, getHeadersFromSelection returns a sanitised list of message headers, including collapsed threads, but only if the front end generated a summary. (If the front end didn't create a summary then .numSelected returns the wrong answer!)

If you just want a sanitised list of selected view indices, use getIndicesForSelection. (Obviously messages in collapsed threads don't have view indices, so that preference doesn't apply here.)

Comment 6

[[e-mail]]

Oh, and these could get very big if you, say, do a select all. So the only reasonably fast way is to fix .numSelected to return zero correctly.

Comment 7

[[e-mail]]

Comment on attachment 504252 [details] [diff] [review]
patch

Or you could just go for the try/catch version.

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2 [Checkin: comment 11]

(In reply to comment #4)
> (In reply to comment #1)
> > We support gFolderDisplay.selectedMessage so one way to fix the bug would be to
> > adapt TB's code for that particular function.
> That doesn't actually work, because our gFolderDisplay.selectedMessage simply
> relies on gDBView.numSelected anyway

Right, should have checked that. Maybe I did and forgot? ;-)

(In reply to comment #7)
> Or you could just go for the try/catch version.

Since I'm not knowledgeable enough to fix the back-end: Here we go.

Comment 9

[[e-mail]]

Comment on attachment 504298 [details] [diff] [review]
patch v2 [Checkin: comment 11]

I don't mind if either of you wants the simpler try/catch version as copied from SelectedMessagesAreJunk() or the other similar methods.

Comment 10

[Karsten Düsterloh]

Comment on attachment 504298 [details] [diff] [review]
patch v2 [Checkin: comment 11]

>+  try {

Brace on its own line please.

>+  catch (ex if (ex.result == Components.results.NS_ERROR_UNEXPECTED))

No need to catch just this single, rather unspecific, exception. 

r=me with that.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 504298 [details] [diff] [review]
patch v2 [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/e5e3779d85b1

Nits addressed upon checkin.