nsImapService::StreamMessage does not return URI
Categories
(Thunderbird :: Add-Ons: Extensions API, defect)
Tracking
(Not tracked)
People
(Reporter: manikulin, Unassigned)
Details
Steps to reproduce:
I am sorry if it was already discussed in
- Bug #1623685
getFullraises an error for IMAP folders - or Bug #1644027
messages.getFullandgetRawfor news message
Some time ago I started development of an extension. I realized thatgetFullandgetRawmethods were severely broken, so had to implement a custom variant of a similar method that callsnsIMsgMessageService::streamMessageusing experiments API. Most annoying bugs have been fixed, but I still use my own method because of it does some additional things useful for me and due to Ubuntu-20.04 LTS focal still provides thunderbird-78. I can reproduce the following using a daily build downloaded a few days ago.
Essentially, I call streamMessage for a message from an IMAP account from an extension using experiments API
Actual results:
I get null in response
tst.callStreamMessage => null
Expected results:
Some URI like for a news account:
tst.callStreamMessage => news://news.gmane.io:119/-tqdnd4ht7mtKIjDnZ2dnUU7-eHNnZ2d%40mozilla.org?group=gmane.comp.mozilla.thunderbird.user&key=15 background.js:9:12
Accordingly to https://hg.mozilla.org/comm-central/file/tip/mailnews/base/public/nsIMsgMessageService.idl#l152
* @return the URL that gets run
Test extension:
background.js
"use strict";
async function testActionHandler(info, tab) {
let selectedMessages = info && info.selectedMessages
&& info.selectedMessages.messages;
for (const message of selectedMessages) {
if (info.menuItemId === "TEST_CALL_STREAM_MESSAGE") {
console.log("tst.callStreamMessage(%o)...", message);
const result = await browser.tst.callStreamMessage(message.id);
console.log("tst.callStreamMessage => %o", result);
} else {
console.error("testActionHandler: unsupported action %o", info.menuItemId);
}
}
}
browser.menus.onClicked.addListener(function testMenuListener(info, tab) {
console.debug("testMenuListener", info, tab);
testActionHandler(info, tab).catch(ex => {
console.error("testMenuListener:", ex);
throw ex;
});
});
async function testMain() {
await browser.menus.removeAll();
browser.menus.create({
contexts: [ "message_list" ],
id: "TEST_CALL_STREAM_MESSAGE",
title: "call StreamMessage",
}, function tstOnMenusCreated() {
const error = browser.runtime.lastError;
if (error) {
console.error("tstOnMenusCreated: %o", error);
}
});
}
testMain();
manifest.json
{
"manifest_version": 2,
"name": "nsIMessageService::streamMessage return value",
"version": "0.1",
"permissions": [ "messagesRead", "menus" ],
"background": { "scripts": [ "background.js" ] },
"experiment_apis": { "tst": {
"schema": "schema.json",
"parent": {
"scopes": [ "addon_parent" ],
"paths": [ [ "tst" ] ],
"script": "experiment.js"
}
} }
}
schema.json
[ {
"namespace": "tst",
"functions": [ {
"name": "callStreamMessage",
"type": "function",
"async": true,
"parameters": [ { "name": "messageId", "type": "integer" } ]
} ]
} ]
experiment.js
"use strict";
var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
var gNoopUrlListener = {
OnStartRunningUrl(aUrl) {
console.log("tstUrlListener.OnStartRunningUrl", aUrl.displaySpec, aUrl);
},
OnStopRunningUrl(aUrl, aExitCode) {
console.log("tstUrlListener.OnStopRunningUrl", aUrl.displaySpec, aExitCode. aUrl);
},
QueryInterface: ChromeUtils.generateQI([Ci.nsIUrlListener]),
};
class PromiseStreamListenerRequestObserver {
constructor() {
this.promise = new Promise((resolve, reject) => {
this._resolve = resolve;
this._reject = reject;
});
this.count = 0;
}
onStartRequest(aRequest) {
console.log("tstPromiseStreamListener.onStartRequest", aRequest);
}
onDataAvailable(aRequest, aInputStream, aOffset, aCount) {
if (this._stream === undefined) {
this._stream = Cc["@mozilla.org/scriptableinputstream;1"].createInstance(
Ci.nsIScriptableInputStream
);
this._stream.init(aInputStream);
}
this.count += aCount;
this._stream.read(aCount);
}
onStopRequest(aRequest, aStatusCode) {
console.log("tstPromiseStreamListener.onStopRequest", aRequest, aStatusCode);
delete this._stream;
try {
if (aStatusCode !== 0) {
Cu.reportError(aStatusCode);
this._reject(new Error("StreamListener error " + aStatusCode));
} else {
this._resolve(this.count);
}
} catch (ex) {
this._reject(ex);
}
}
}
PromiseStreamListenerRequestObserver.prototype.QueryInterface
= ChromeUtils.generateQI([Ci.nsIStreamListener]);
var tst = class extends ExtensionCommon.ExtensionAPI {
getAPI(context) {
return { tst: {
async callStreamMessage(messageId) {
let messenger = Cc["@mozilla.org/messenger;1"].createInstance(
Ci.nsIMessenger
);
const msgHdr = context.extension.messageManager.get(messageId);
let msgUri = msgHdr.folder.generateMessageURI(msgHdr.messageKey);
let service = messenger.messageServiceFromURI(msgUri);
const streamListener = new PromiseStreamListenerRequestObserver();
const uri = service.streamMessage(
msgUri,
streamListener,
null, // msgWindow,
gNoopUrlListener,
false, // convertData,
"", // additionalHeader
false // localOnly
);
console.log("tst: streamMessage returned", uri && uri.displaySpec, uri);
console.log("tst: length", await streamListener.promise);
return uri && uri.displaySpec;
},
}, };
}
onShutdown(isAppShutdown) {
if (!isAppShutdown) {
Services.obs.notifyObservers(null, "startupcache-invalidate", null);
}
}
};
I forgot to mention a couple of things. Initially it was quite obscure and before I had found a workaround, I added a check for StreamMessage return value with hope to avoid hangs when no data fetched for messages. It worked since mostly I am interested in mail list messages exposed through news.gmane.io NNTP server. Later I realized that check for URI is a false alarm for IMAP messages when content is available despite null return value.
My point is that implementation and specification should be consistent. I can not judge which one should be adjusted.
|
||
Comment 2•4 years ago
|
||
Not sure there's a bug here. The .idl is what it is... which returns an nsIURI. But that is not something an add-on should be given of course...
|
||
Comment 3•4 years ago
|
||
What is missing in the WebExtension API, that you still need to use an Experiment?
(In reply to John Bieling (:TbSync) from comment #3)
What is missing in the WebExtension API, that you still need to use an Experiment?
Actually I need headers decoded from e.g. Quoted-Printable representation, but with e.g. raw dates, not Date object, so original time zone is available.
const headers = MimeParser.extractHeaders(rawText);
Maybe headers field of messages.getFull provides the same, but it is unreliable in thunderbird-78 while I prefer to stay with system package. So I need messages.getHeaders method that returns semi-raw values, it would be great if message body is not fetched at all for messages absent in local cache.
I have a couple of other ideas, but they are still ideas without implementation yet and I do not think it is appropriate to discuss them here. Maybe I create feature requests later (custom scheme handlers that can be invoked from command line and ability to display particular message found in some folder).
(In reply to Magnus Melin [:mkmelin] from comment #2)
Not sure there's a bug here. The .idl is what it is... which returns an nsIURI. But that is not something an add-on should be given of course...
Originally I tried to check return value with hope that non-null value increases probability that callbacks of passed listeners will be properly invoked, see e.g. Bug #1695235.
|
|
||
Comment 5•4 years ago
|
||
I am sorry max, I am not able to extract a clean feature request from your last comment.
Maybe headers field of messages.getFull provides the same, but it is unreliable in thunderbird-78
TB78 is EOL, could you use TB Beta as your base for requesting missing features?
So I need messages.getHeaders method that returns semi-raw values
What is a semi-raw value? Could you give examples what you currently get from the API and what you need?
it would be great if message body is not fetched at all for messages absent in local cache.
Can you state your use-case? If I request getFull(), I want the full message, or not?
(In reply to John Bieling (:TbSync) from comment #5)
Maybe headers field of messages.getFull provides the same, but it is unreliable in thunderbird-78
TB78 is EOL, could you use TB Beta as your base for requesting missing features?
The reason why I am keeping compatibility with Thunderbird-78 is the following:
https://packages.debian.org/search?keywords=thunderbird
bullseye (stable) 1:78.14.0-1~deb11u1
https://packages.ubuntu.com/search?keywords=thunderbird
focal-updates 1:78.14.0+build1-0ubuntu0.20.04.2 (Ubuntu-20.04 focal is the latest long time support release)
So I need messages.getHeaders method that returns semi-raw values
What is a semi-raw value? Could you give examples what you currently get from the API and what you need?
As semi-raw value I mean UTF-8 strings decoded from 7-bit transfer encoding (quoted printable or base64 fragments) in names but e.g. "Date" header is not converted to JS Date object as in messages.get return value. It is impossible to get original time zone from Date objects, that is why I prefer original value. Participants of mail list discussion may reside in different timezones, so local time of one user means nothing for others when mentioned in a message.
I have tried some build of thunderbird-96 downloaded earlier. It seems, headers.from and headers.date fields of messages.getFull return value contain what I am expecting to get. I have not tried various edge cases though, so can not confirm that it is reliable enough.
it would be great if message body is not fetched at all for messages absent in local cache.
Can you state your use-case? If I request getFull(), I want the full message, or not?
I mean an interface to streamHeaders (if it works for IMAP and NNTP) from
https://hg.mozilla.org/comm-central/file/tip/mailnews/base/public/nsIMsgMessageService.idl#l179
as an option to save traffic.
|
|
||
Comment 7•4 years ago
|
||
Any request for TB78 is a wontfix, as we do not release updates. If your date issue is not resolved in Beta, please create a new bug for (only) that.
This bug has been moved into the WebExtension component by Magnus, but I am not sure what to do with it. I still do not see a request to improve our WebExtension APIs. Do you want to change messages.getRaw() / messages.getFull() to early exit and not return anything if the message is not cached?
In general, it is best to create single-request-bugs.
(In reply to John Bieling (:TbSync) from comment #7)
This bug has been moved into the WebExtension component by Magnus, but I am not sure what to do with it. I still do not see a request to improve our WebExtension APIs. Do you want to change messages.getRaw() / messages.getFull() to early exit and not return anything if the message is not cached?
In general, it is best to create single-request-bugs.
I have filed this bug because nsImapService::StreamMessage method does not follow documentation from the .idl file.
It was you who asked me if I had feature requests related to extensions. I suggested an interface for streamHeaders that could be named e.g. messages.getFullHeaders(). Maybe some options could be passed to getRaw and getFull instead. Fetching headers might be different from failure when message is not cached yet.
I noticed the problem with nsImapService::StreamMessage because when I started my experiments messages.getFull was broken for NNTP and IMAP accounts. Currently I am unaware of real problems with getFull in thunderbird-96 (maybe even in thunderbird-91), but I am not ready to drop earlier created workaround due to thunderbird versions available in stable linux distributions. This bug is not blocker for me. I just expect that behavior of methods should be consistent with interface documentation.
|
|
||
Comment 9•4 years ago
•
|
||
It was you who asked me if I had feature requests related to extensions.
That is true. My bad. I should not have done that. Sorry for causing the confusion.
Magnus said the IDL is what it is. Closing this as invalid.
|
|
||
Updated•4 years ago
|
Description
•