1085562 - TEST-UNEXPECTED-FAIL | ../../../resources/logHelper.js | Error console says [stackFrame [Exception... "Failure arg 0 [nsITreeView.setTree]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: /builds/slave/test/build/tests/xpcshell/tests/mai

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Joshua Cranmer [:jcranmer]]

Attachment: Don't mock nsITreeBoxObject

We don't necessarily need the tree box object... so let's tweak some of the tests to make this not necessary.

Comment 1

[Joshua Cranmer [:jcranmer]]

Attachment: Don't mock nsITreeBoxObject

It helps to rerun all mailnews tests first.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8508144 [details] [diff] [review]
Don't mock nsITreeBoxObject

Review of attachment 8508144 [details] [diff] [review]:
-----------------------------------------------------------------

I totally agree with Joshua.
I had also tried to remove the mock, and I gave up because it's too hard to make all mozmill tests pass.
But at least xpcshell test, the nsITreeBoxObject mock is not necessary at all. 

This patch will make all xpcshell tests pass.

::: mailnews/base/src/nsMsgDBView.cpp
@@ +2559,5 @@
>    case nsMsgViewCommandType::collapseAll:
>      rv = CollapseAll();
>      m_viewFlags &= ~nsMsgViewFlagsType::kExpandAll;
>      SetViewFlags(m_viewFlags);
>      NS_ASSERTION(mTree, "no tree, see bug #114956");

This assertion should be also removed? This is the last assertion for mTree.

Comment 3

[Kent James (:rkent)]

Comment on attachment 8508144 [details] [diff] [review]
Don't mock nsITreeBoxObject

Review of attachment 8508144 [details] [diff] [review]:
-----------------------------------------------------------------

r+=me but please incorporate hiro's suggestion to eliminate the extra assertion

::: mailnews/base/src/nsMsgDBView.cpp
@@ +2559,5 @@
>    case nsMsgViewCommandType::collapseAll:
>      rv = CollapseAll();
>      m_viewFlags &= ~nsMsgViewFlagsType::kExpandAll;
>      SetViewFlags(m_viewFlags);
>      NS_ASSERTION(mTree, "no tree, see bug #114956");

Yes I agree, those were debugging to find a crash a long time ago.

Comment 4

[Kent James (:rkent)]

(In reply to Kent James (:rkent) from comment #3)
> Comment on attachment 8508144 [details] [diff] [review]
> Don't mock nsITreeBoxObject
> 
> Review of attachment 8508144 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+=me but please incorporate hiro's suggestion to eliminate the extra
> assertion
> 
> ::: mailnews/base/src/nsMsgDBView.cpp
> @@ +2559,5 @@
> >    case nsMsgViewCommandType::collapseAll:
> >      rv = CollapseAll();
> >      m_viewFlags &= ~nsMsgViewFlagsType::kExpandAll;
> >      SetViewFlags(m_viewFlags);
> >      NS_ASSERTION(mTree, "no tree, see bug #114956");
> 
> Yes I agree, those were debugging to find a crash a long time ago.

That comment was supposed to be added after hiro's comment, which is how they appeared in the review diff

Comment 5

[Joshua Cranmer [:jcranmer]]

https://hg.mozilla.org/comm-central/rev/ca3044b26a30