Closed Bug 1694988 Opened 4 years ago Closed 1 year ago

nsNntpService::StreamMessage does not support aConvertData argument

Categories

(MailNews Core :: Networking: NNTP, defect)

defect

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: manikulin, Unassigned)

Details

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

Try to create an extension for thunderbird that extracts some information from a news message using browser.messages.getFull(id) API function, see https://bugzilla.mozilla.org/show_bug.cgi?id=1644027#c2 for an example extension and for the bug on extensions API.

Actual results:

Hanged promise. Various errors in console that should be subject of dedicated bug reports due to insufficient error handling.

Expected results:

It should be possible to work with NNTP messages just as with IMAP, local or RSS messages.

Particular problem with messages.getFull(id) function from extensions API is that nsNntpService::StreamMessage() method implementation of the nsIMsgMessageService interface ignores aConvertData argument, see https://hg.mozilla.org/comm-central/file/tip/mailnews/news/src/nsNntpService.cpp#l1333 however it is used by JS module https://hg.mozilla.org/comm-central/file/tip/mailnews/db/gloda/modules/MimeMessage.jsm#l241 invoked from the implementation of WebExtensions API https://hg.mozilla.org/comm-central/file/tip/mail/components/extensions/parent/ext-messages.js#l212

It is quite sour that messages.getRaw(id) method from extensions API is broken for NNTP messages as well due to another peculiarity of nsNntpService implementation.

Summary: nsNntpService::StreamMessage does support aConvertData argument → nsNntpService::StreamMessage does not support aConvertData argument

Should we put this in the addons component?

Flags: needinfo?(john)

It is quite sour that messages.getRaw(id) method from extensions API is broken for NNTP messages as well due to another peculiarity of nsNntpService implementation.

I think getRaw() and getFull() are working for news now. Max?

Flags: needinfo?(john)

(In reply to Wayne Mery (:wsmwk) from comment #1)

Should we put this in the addons component?

This bug was filed namely for the nsNntpService class. For add-ons there were e.g. Bug #1644027, Bug #1713145, Bug #1697075.

I think, some problems described in that bugs might be avoided if nsNntpService were not broken and did not violate API contracts. I was completely confused by decisions related to the Bug #1743425. It was created to fix discrepancy between documentation and real behavior of a C++ class but the issue was at first moved to add-ons and later closed as invalid there. I can only say: good luck with such approach, but certainly it is not a way to the robust and reliable application.

I am unaware of background behind such decisions. Maybe your are going to throw away C++ code completely. When I tried extensions API a year ago, it was completely broken due to inconsistencies between low level code and JS interfaces. Something in NNTP has been rewritten to JS since that time, but due to e.g. Bug #1753195 and Bug #1757907 I suspect serious problems in communication with the lower network layer, not to mention Bug #1757904, Bug #1758100. And these bugs directly affects users, not add-on developers.

(In reply to John Bieling (:TbSync) from comment #2)

It is quite sour that messages.getRaw(id) method from extensions API is broken for NNTP messages as well due to another peculiarity of nsNntpService implementation.

I think getRaw() and getFull() are working for news now. Max?

I am unaware of problems with message.getRaw() and message.getFull() mostly because in my extension I still use workarounds created earlier using experiments API. I have realized that I still can not drop experiments API from my extension despite at first glance getRaw and getFull works well.

Actually after update of thunderbird-78 to thunderbird-91 I met serious user-level problems so I postponed my activity with an extension.

nsNNTP code no longer exists since nntp-js. If the news behavior is still flawed, please file a new bug report.

Status: UNCONFIRMED → RESOLVED
Closed: 1 year ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.