210824 - In <msgMail3PaneWindow.js>, "Error/Exception ... nsIRDFService.GetResource ... SelectFolder"

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Serge Gautherie (:sgautherie)]

User-Agent:       Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4) Gecko/20030624

That's v1.4rc3.

I selected the MWFE component by default: feel free to change it.


Reproducible: Didn't try

Steps to Reproduce:

Actual Results:  
{
Error: [Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIRDFService.GetResource]"  nsresult: "0x80070057
(NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://messenger/content/msgMail3PaneWindow.js :: SelectFolder :: line 1421" 
data: no]
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1421
}


I don't know if this bug has any bad side-effect,
besides cluttering the JS.C..

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6) Gecko/20040113] (W98SE)

The line number changed, but (seldom !?) bug still there
{
Error: [Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIRDFService.GetResource]"  nsresult: "0x80070057
(NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://messenger/content/msgMail3PaneWindow.js :: SelectFolder :: line 1418" 
data: no]
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1418
}
Code is
{{
function SelectFolder(folderUri)
{
    var folderTree = GetFolderTree();
    var folderResource = RDF.GetResource(folderUri);     <-- 1418 !
    var msgFolder =
folderResource.QueryInterface(Components.interfaces.nsIMsgFolder);

    // before we can select a folder, we need to make sure it is "visible"
    // in the tree.  to do that, we need to ensure that all its
    // ancestors are expanded
    var folderIndex = EnsureFolderIndex(folderTree.builderView, msgFolder);
    ChangeSelection(folderTree, folderIndex);
}
}}

I started to load a page in the Browser;
I double clicked on the MailNews/NewMailsArrived icon to (re)open/launch MailNews;
got this error.

Updating:
*(O) W95 -> W98, since I upgraded.

Comment 2

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6) Gecko/20040113] (W98SE)

Again:
double clicking the MewMessages icon:
MailNews was already opened, and got focused;
got the exception.
(icon stayed in the tray bar).

I have installed Attachment #139690 [details] [diff] for Bug #189430, which made no improvement.
(as expected)

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: ScreenShot of comment 2 display

At this time, (I have not touch anything in MailNews)
I can reproduce the Exception at will !

Comment 4

[james davis]

Same for:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/2004032604


Error: [Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIRDFService.GetResource]"  nsresult: "0x80070057
(NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://messenger/content/msgMail3PaneWindow.js :: SelectFolder :: line 1433" 
data: no]
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1433

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> Same for:
> Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/2004032604

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040316] (W98SE)

Confirming comment 2:

When opening MailNews using Ctrl-2 from Browser:
no exception, and |folderUri == mailbox://[e-mail]/Inbox|

When double-clicking the NewMessages traybar icon:
exception, and |folderUri == null| <-- !!

I'll try and see where this |null| comes from...

seth:
Any insight on the cause/solution ?

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> When double-clicking the NewMessages traybar icon:
> exception, and |folderUri == null| <-- !!
> 
> I'll try and see where this |null| comes from...

(Remember: MailNews window must already be opened/loaded.)

Calling(-1) code is
mailWindow.js/.SelectFolder/469
{{
nsMsgWindowCommands.prototype =
{

  // {...}

  SelectFolder: function(folderUri)
  {
    SelectFolder(folderUri);     // <-- line 469 : |folderUri == null|
  },
}}

Then, where is this called(-2) from ? (a .cpp file !?)

neil:
seth is in "exclude mode" for this bug comments;
Any insight on the cause/solution ?

Comment 7

[[e-mail]]

When you double-click the tray icon it opens or focuses the mail window. However
the focusing code also tries to switch to a folder; the popup alert window uses
this to load the folder with the new messages, but the icon code does not know
which folder to load :-(

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> the icon code does not know
> which folder to load :-(

Then, <nsMessengerWinIntegration.cpp>
{{
243     static long CALLBACK WindowProc( HWND msgWindow, UINT msg, WPARAM wp,
LPARAM lp ) 
244     {
251            openMailWindow(mailName.get(), nsnull);     <-- |nsnull| !
256     }
}}

calls
{{
124 static void openMailWindow(const PRUnichar * aMailWindowName, const char *
aFolderUri)
125 {
131     if (domWindow)
132     {
139         if (msgWindowCommands)
140           msgWindowCommands->SelectFolder(aFolderUri);
144     }
145     else
146     {
152         messengerWindowService->OpenMessengerWindowWithUri("mail:3pane",
aFolderUri, nsMsgKey_None);
153     }
155 }
}}

In the latter case: (in <nsMessengerBootstrap.cpp>)
{{
103 NS_IMETHODIMP nsMessengerBootstrap::OpenMessengerWindowWithUri(const char
*windowType, const char * aFolderURI, nsMsgKey aMessageKey)
104 {
114   if (aFolderURI)     <-- Protected against |nsnull| argument !
115   {
119     scriptableFolderURI->SetData(nsDependentCString(aFolderURI));
126   }
140 }
}}

But, in the former case: (see comment 6)
|SelectFolder| is not protected against |null| argument !

Which fix is needed:
Simply protect against |null| (then at which code place ?) ? (This I could do)
Or add new code to pass the proper folder in the first place ? (This I could not do)

Comment 9

[[e-mail]]

Resummarizing for the benefit of the CC'd module owners:
When biff fires an alert it provides an alert cookie which is the URL of the
folder to open if the alert is clicked. If the alert is ignored and the tray
icon is created then the tray icon no longer knows the URL of the folder, so it
calls openMailWindow without a URL. This null value then gets passed around
until it reaches RDF.GetResource which throws an exception. So the question is
does the tray icon need to be smarter or does openMailWindow need a null check?

Comment 10

[Scott MacGregor]

Good question Neil. I think the answer to both your questions is Yes :)

1) The biff icon MessageWindow
(http://lxr.mozilla.org/mozilla/source/mailnews/base/src/nsMessengerWinIntegration.cpp#158)
should store the folderURI so it can pass it in to openMailWindow instead of
passing in nsnull

2) openMailWindow should also have a null check

Comment 11

[[e-mail]]

OK, so it looks as if Serge has volunteered to do 2), once that's done we'll
need  a new bug for 1) (if there isn't one already, I haven't checked).

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-w) <nsMessengerWinIntegration.cpp> (for review only) [Check-in: See patch Av1-ci]

(In reply to comment #10)
> 2) openMailWindow should also have a null check

Does this do it ?

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 145434 [details] [diff] [review]
(Av1-w) <nsMessengerWinIntegration.cpp> (for review only)
[Check-in: See patch Av1-ci]


I have no compiler: Could you compile/test/review it ? Thanks.

Comment 14

[[e-mail]]

Comment on attachment 145434 [details] [diff] [review]
(Av1-w) <nsMessengerWinIntegration.cpp> (for review only)
[Check-in: See patch Av1-ci]

Sorry for taking so long, but this file has other bugs in it that I had to fix
before I could do the review...

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 145434 [details] [diff] [review]
(Av1-w) <nsMessengerWinIntegration.cpp> (for review only)
[Check-in: See patch Av1-ci]


'approval1.7=?':
Trivial U.I. code fix, no risk.

Comment 16

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-ci) <nsMessengerWinIntegration.cpp> (for check-in only) [Checked in: Comment 17]

Same as Av1w, but has white space diff. too.

Comment 17

[[e-mail]]

Fix checked in.

Comment 18

[:deanis74]

Comment on attachment 145752 [details] [diff] [review]
(Av1-ci) <nsMessengerWinIntegration.cpp> (for check-in only)
[Checked in: Comment 17]

>+  else
>+  {
>+    // the user doesn't have a mail window open already so open one for them...
>+    nsCOMPtr<nsIMessengerWindowService> messengerWindowService =
>+      do_GetService(NS_MESSENGERWINDOWSERVICE_CONTRACTID);
>+    // if we want to preselect the first account with new mail,
>+    // here is where we would try to generate a uri to pass in
>+    // (and add code to the messenger window service to make that work)
>+    if (messengerWindowService)
>+      messengerWindowService->OpenMessengerWindowWithUri(
>+                                "mail:3pane", aFolderUri, nsMsgKey_None);
>   }
> }

So you don't need to null-check aFolderUri here?

Comment 19

[[e-mail]]

No, a null folder there means to load the default folder (as if you had opened
mail for the first time).

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> we'll
> need  a new bug for 1) (if there isn't one already, I haven't checked).

I did 1 BugZilla search for "MailNews; title = biff", didn't find this request;
created new bug 240836 !