1632847 - Port bug 1632021 to Thunderbird (Avoid QI between transaction classes)

Status: RESOLVED FIXED

(MailNews Core :: Backend, task, P2)

Description

[Magnus Melin [:mkmelin]]

+++ This bug was initially created as a clone of Bug #1632021 +++

Due to changes in bug 1632021
https://hg.mozilla.org/mozilla-central/rev/9e2e2b767d83
https://hg.mozilla.org/mozilla-central/rev/a4a7fce9f73e
https://hg.mozilla.org/mozilla-central/rev/946dc2965175
https://hg.mozilla.org/mozilla-central/rev/55abc248f524

... builds are broken with
[task 2020-04-24T11:09:30.594Z] 11:09:30 ERROR - /builds/worker/checkouts/gecko/comm/mailnews/base/util/nsMsgDBFolder.cpp:1361:48: error: allocating an object of abstract class type 'nsMsgReadStateTxn'
[task 2020-04-24T11:09:30.594Z] 11:09:30 INFO - RefPtr<nsMsgReadStateTxn> readStateTxn = new nsMsgReadStateTxn();
[task 2020-04-24T11:09:30.594Z] 11:09:30 INFO - ^
[task 2020-04-24T11:09:30.594Z] 11:09:30 INFO - /builds/worker/workspace/obj-build/dist/include/nsITransaction.h:53:14: note: unimplemented pure virtual method 'GetAsEditTransactionBase' in 'nsMsgReadStateTxn'
[task 2020-04-24T11:09:30.594Z] 11:09:30 INFO - NS_IMETHOD GetAsEditTransactionBase(mozilla::EditTransactionBase * * _retval) = 0;
[task 2020-04-24T11:09:30.594Z] 11:09:30 INFO - ^
[task 2020-04-24T11:09:30.595Z] 11:09:30 INFO - 1 error generated.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1632847_msgtxn.patch

Going to land this as bustage fix as it's at least building on linux. Things seem to be working, but I don't know if it's supposed to do something more than NS_ERROR_NOT_IMPLEMENTED.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/531719d26930
Port bug 1632021 to Thunderbird (Avoid QI between transaction classes). rs=bustage-fix

Comment 3

[Ben Campbell]

Comment on attachment 9143090 [details] [diff] [review]
bug1632847_msgtxn.patch

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

Looks fine to me.
C-C doesn't seem to derive (or refer to) `EditTransactionBase` anywhere.
Just to sanity-check: a cursory look at `nsMsgTxn` seems to indicate it's for editing various metadata/properties on messages, rather than for the message editor, right?
(I don't know much about the editing in either M-C or C-C, but it might be worth taking a look at the C-C editing and sanity-check the patterns against what editing in M-C looks like... eg maybe the message editing in C-C _should_ be making use of `EditTransactionBase` for editing operations... I really don't know :-)

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: 9/18-9/27)]

Comment on attachment 9143090 [details] [diff] [review]
bug1632847_msgtxn.patch

Ah, sorry, I didn't realize that there is another C++ implementation of `nsITransaction` in comm-central because I checked JS part mainly. This is right fix for the purpose of the method.

Comment 5

[Magnus Melin [:mkmelin]]

Thanks!
Indeed not used for editing message content, it's for things like undo delete.