1598332 - WX API for marking a mail as spam or ham

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

[Marvin]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.108 Safari/537.36

Steps to reproduce:

I tried marking a mail as spam

Actual results:

nothing since it cant be done as of now

Expected results:

a function which allows the webextension to mark a mail as spam

Comment 1

[Marvin]

Attachment: ext_messages_schema.json

Comment 2

[Marvin]

Attachment: ext-messages.js

Comment 3

[Marvin]

Fallen was guiding me to extend the possible options in the update function...
But I dont know how to build a patch out of it...

Comment 4

[Marvin]

Attachment: test_ext_messages.js

I dont know if thats the way its wanted but I tried adding some test cases...
modified rows: 117, 128-130, 150, 178-180

Comment 5

[Magnus Melin [:mkmelin]]

Marvin, can you submit it these as a patch?

Comment 6

[Marvin]

Attachment: the changes made as a patch

Comment 7

[Marvin]

Attachment: diff file made after the commit

Comment 8

[Geoff Lankow (:darktrojan)]

Comment on attachment 9111568 [details] [diff] [review]
diff file made after the commit

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

This is good! It raises some questions though, that I don't know the answer to:

* If we mark a message as junk, should we automatically mark it as read like the UI does? (depending on the user's prefs)
* What about moving it to the junk folder automatically? (depending on the user's prefs)
* Same questions, but in reverse, if we mark a message as not junk.
* On my IMAP server, if I mark a message as both not junk and not read, only one applies. We need to figure out what's going on with that.

::: mail/components/extensions/parent/ext-messages.js
@@ +273,5 @@
>              msgHdr.markFlagged(newProperties.flagged);
>            }
> +          if (newProperties.junk !== null) {
> +            let messages = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
> +            let score = newProperties.junk ? Ci.nsIJunkMailPlugin.IS_SPAM_SCORE : Ci.nsIJunkMailPlugin.IS_HAM_SCORE;

These two lines are too long. Run these commands to set up the linter/formatter and run it on this file.

mach eslint --setup
mach eslint --fix mail/components/extensions/parent/ext-messages.js

::: mail/components/extensions/test/xpcshell/test_ext_messages.js
@@ +175,5 @@
>    ok(message.isRead);
>    extension.sendMessage();
>  
> +  await extension.awaitMessage("junk");
> +  ok(message.isJunk);

This line should be "equal(message.getStringProperty("junkscore"), 100);"

Further down, where we check the flags have been cleared, the same check should take place, but with 0 instead of 100.

Comment 9

[Geoff Lankow (:darktrojan)]

Magnus, your thoughts on comment 8's questions?

Comment 10

[Geoff Lankow (:darktrojan)]

(In reply to Geoff Lankow (:darktrojan) from comment #8)

This line should be "equal(message.getStringProperty("junkscore"), 100);"

On reflection, getProperty would probably be more appropriate!

Comment 11

[Magnus Melin [:mkmelin]]

Good question... in general APIs should not cause such side effects, but do exactly what you say and nothing more. That does raise the question of how to get the side effects for the cases when you do want them. Maybe that needs to be a separate API like processMessage()?

Comment 12

[Marvin]

maybe I can give some perspektive to the raised questions from my point of view:

• If we mark a message as junk, should we automatically mark it as read like the UI does? (depending on the user's prefs)
  No, I think that because it (most likely) willl be set by an extension the user should take notice of that mail because the extension might be wrong

• What about moving it to the junk folder automatically? (depending on the user's prefs)
  No, same point as above and because I just look roughly through the spam folder and then press "delete all"

• Same questions, but in reverse, if we mark a message as not junk.
  If we mark it as not spam to mark it as unread too? only thing that comes to mind would be to get the attention of the user... but if said mail is still in the spam folder it wouldnt make much difference...

• On my IMAP server, if I mark a message as both not junk and not read, only one applies. We need to figure out what's going on with that.
  agreed

Comment 13

[Marvin]

Attachment: patch.diff

Comment 14

[Geoff Lankow (:darktrojan)]

Attachment: 1598332-spam-or-ham.diff

Patches combined, white-space fixed. I think this is good to go.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e1257b911bd1
Extend messages.update API function to mark messages as junk or not junk. r=darktrojan DONTBUILD

Comment 16

[Marvin]

worked fine for a couple of days but now the spam symbol just lights up for a second then vanishes again...

Comment 17

[Geoff Lankow (:darktrojan)]

Bug 1602628 should fix this.

Comment 18

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

(In reply to Geoff Lankow (:darktrojan) from comment #8)

• If we mark a message as junk, should we automatically mark it as read like
  the UI does? (depending on the user's prefs)
• What about moving it to the junk folder automatically? (depending on the
  user's prefs)
• Same questions, but in reverse, if we mark a message as not junk.

Personally I think this shouldn't automatically happen if an extension does it. What we can do is expose the setting if junk emails should be marked read or moved to the junk folder, then let the extension handle it. This might be a few calls more, but allows for more use cases where the moving or marking read might not be intended.

Either that, or maybe some flag that controls if such automatic actions should happen.

Comment 19

[Geoff Lankow (:darktrojan)]

Attachment: 1598332-spam-or-ham-esr.diff

This has been in beta since 73. Let's move it on.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a33fdff0980f8bad7810a1ca85f51a041e3deabc

Comment 20

[Wayne Mery (:wsmwk)]

Comment on attachment 9131420 [details] [diff] [review]
1598332-spam-or-ham-esr.diff

Approved for ESR

Comment 21

[Magnus Melin [:mkmelin]]

Thunderbird 68.7: https://hg.mozilla.org/releases/comm-esr68/rev/fa017d488bd07cc3547dfaf2cd145a923889eff5