1556242 - unable to type in Compose Window when forwarding; 0x80004005 (NS_ERROR_FAILURE) [nsIIOService.newURI]"

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Calum Mackay]

With current Daily, when forwarding a message (as attachment, non-HTML email), it's not possible to type in the compose window.

I notice that a new paperclip glyph is showing for the attachment area on the RHS, which is new since the last working build.

Last working build I have: Daily 0527

First bad build i have: Daily 0530

and broken thereafter up to and including Daily 0602

I'll try to narrow that down further.

NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface] stack-trace-collector.js:75
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIIOService.newURI]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: AddAttachments :: line 4408" data: no] MsgComposeCommands.js:4408:29
AddAttachments chrome://messenger/content/messengercompose/MsgComposeCommands.js:4408
ComposeStartup chrome://messenger/content/messengercompose/MsgComposeCommands.js:2911
ComposeLoad chrome://messenger/content/messengercompose/MsgComposeCommands.js:3035
onload chrome://messenger/content/messengercompose/messengercompose.xul:1
NS_ERROR_NOT_AVAILABLE: Cannot call openModalWindow on a hidden window Prompter.jsm:345
NS_ERROR_NOT_AVAILABLE: Cannot call openModalWindow on a hidden window Prompter.jsm:345
openModalWindow resource://gre/modules/Prompter.jsm:345
openPrompt resource://gre/modules/Prompter.jsm:566
alert resource://gre/modules/Prompter.jsm:615
alert resource://gre/modules/Prompter.jsm:55
ComposeLoad chrome://messenger/content/messengercompose/MsgComposeCommands.js:3038
onload chrome://messenger/content/messengercompose/messengercompose.xul:1

Comment 1

[Calum Mackay]

MacOS 10.14.5

Comment 2

[Calum Mackay]

can't narrow further: there are no builds on ftp.m.org between 0527 & 0530.

Comment 3

[Jorg K (CEST = GMT+2)]

Yes, I can reproduce this. It doesn't work for forwarding an IMAP message, works OK for a message from a local folder. Looks like a fallout from bug 1550945, first parts landed on 2019-05-30. Thanks for reporting, I'll take a look.

Comment 4

[Jorg K (CEST = GMT+2)]

OK, the URL is something like:
imap-message://[e-mail]/INBOX/spambucket#14321
I don't see why that doesn't work.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: 1556242-discard-message.patch

This fixes the issue. No idea why that was working differently before.

Comment 6

[Jorg K (CEST = GMT+2)]

Oh, for the reviewer. The issue is that nsImapService::NewURI() calls nsImapService::GetServerFromUrl() and that calls FindServerByURI() and that doesn't find anything on the imap-message URL.

Comment 7

[:aceman]

Comment on attachment 9069814 [details] [diff] [review]
1556242-discard-message.patch

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

Thanks, works for me.

What URI are we trying to look up in this case? Is it from the original message (the attached one) to detect in which account we are to set up the sending identity, etc.?

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +1727,5 @@
>  
>    nsAutoCString type;
>    rv = aURI->GetScheme(type);
> +  if (StringEndsWith(type, nsDependentCString("-message")))
> +    type.SetLength(type.Length() - 8);

Please add a comment that we strip the '-message' suffix here. Or would it be better to strip it at the caller before it entering the nsMsgAccountManager::FindServerByURI() ?

Comment 8

[Jorg K (CEST = GMT+2)]

It's not a lookup. "New URI" constructs a new nsIURI object, in this case an nsIImapUrl, for a spec. The spec has the scheme "imap-message" which is the URI of the attached message. It's turned into an object here:
https://searchfox.org/comm-central/rev/66bdc4565824a8c59d34ba88c3d8e70448356a71/mail/components/compose/content/MsgComposeCommands.js#4408 - reported in comment #0.

I looked at the callers. They pass around the nsIURI object, so in order to strip the "-message", we'd have to create a new nsIURI object with the the modified scheme. It's easier to do when the spec is again extracted as a string.

Besides, nsMsgAccountManager::FindServerByURI() should really be equipped to remove/convert undesired suffixes.

The big question is: Why is this required now. How was the nsIURI constructed before bug 1550945 and what was passed to nsMsgAccountManager::FindServerByURI() in that case. That's not an easy question to answer since it would mean reverting the local build to before that bug to debug it.

Comment 9

[Jorg K (CEST = GMT+2)]

OK, going back to C-C rev c999415db730, M-C d8321d0b2c5b43933c5e9f201934ec7d4c from
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=c999415db730294e7cc75c8e509ad1818695978c
to investigate. Not fun.

Comment 10

[Jorg K (CEST = GMT+2)]

OK, I added debug in nsImapService::NewURI() and it doesn't get called on the imap-message spec. Hmm, so which protocol handler handles it?

Comment 11

[Jorg K (CEST = GMT+2)]

Well, nsMailboxService::NewURI() doesn't handle it either. imap-message isn't really a registered scheme, those *-message URLs are some sort of hack. IIRC Kent tried to work out what's behind them, but to my knowledge, he didn't document it.

As it stands, I think we need to go with the patch here, see reasoning in comment #8.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1556242-discard-message.patch

Added comment and shifted the code into the if block.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2be4f664709c
Discard '-message' part of URL scheme in nsMsgAccountManager::FindServerByURI(). r=aceman DONTBUILD

Comment 14

[Calum Mackay]

thanks very much indeed, Jörg

Comment 15

[Jorg K (CEST = GMT+2)]

Turned out that the fix was wrong, anyway, the right fix will be in bug 1556981.

EDIT: Spoke too early, looks like we need this fix after all.

Comment 16

[Calum Mackay]

Interesting, thanks.

Regardless, with 0606 my problem here is fixed, and I don't see any of the problems mentioned in bug 1556981, other than a paperclip icon not a TB icon (see attached).

Comment 17

[Calum Mackay]

Attachment: see comment 16

Comment 18

[Jorg K (CEST = GMT+2)]

That icon is wrong and will be fixed in bug 1556981. That bug also has attachments showing the correct icon (attachment 9069929 [details]).