926056 - Attachment reminder: User's explicit "Remind me later" request from notification not honoured if attachment keywords no longer in body: No alert dialogue when sending

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Thomas D. (:thomas8)]

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

STR (tested on TB24,WinXP,POP3 account)

1) compose message
2) type attachment reminder keyword into msg body, e.g. "attachment"
3) from attachment notification bar, click "Remind me later"
4) remove attachment reminder keyword(s) from body: delete the word "attachment"
5) Send msg (or Send later: Ctrl+Shift+Enter, for testing purposes)

Actual result

5) "Attachment Reminder" alert dialogue does NOT appear, inspite of user's explicit request "Remind me later" in step 3. Technically, this happens in http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js?rev=8dff78580066#2401, which structurally works like this:

Alert if ... && ShouldShowAttachmentNotification(...), where ShouldShowAttachmentNotification(...) returns true if attachmentbucket.itemcount == 0 AND attachments keywords are found.

Expected result

5) "Attachment Reminder" alert dialogue should appear, as explicitly requested by user in step 3 when he clicked "Remind me later". We need to honour that instruction regardless of body text content. If the exact words which originally triggered the notification bar are still present or not when sending is entirely irrelevant for the user's declared wish to be reminded about adding attachments.

* This bug is ux-mode-error as interface/behaviour is in an unexpected state (no reminder).
* Datalossy (at least) as users relying on this feature can be tricked into sending their messages without attachments against their declared will. Depending on scenario, real-world consequences can be major e.g. if users fail to attach important business documents, resumes etc.

We could probably fix this easily in pending patch for bug 521158.

Comment 1

[Thomas D. (:thomas8)]

I contributed the code change which will fix this in bug 521158 (fixed by Suyash).
So I'll assign this to myself ;)

We've streamlined the conditions quite radically:
+    if (gManualAttachmentReminder || (getPref("mail.compose.attachment_reminder_aggressive") &&
+         document.getElementById("attachmentNotificationBox").currentNotification))

The behaviour is still mostly the same (only better ;)), because .currentNotification implies all the other conditions previously explicit here. New code is faster, too :)

Comment 2

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #1)
> I contributed the code change which will fix this in bug 521158 (fixed by
> Suyash). So I'll assign this to myself ;)
> 
> We've streamlined the conditions quite radically:
> +    if (gManualAttachmentReminder || (getPref("mail.compose.attachment_reminder_aggressive") &&
> +        document.getElementById("attachmentNotificationBox").currentNotification))
> 
> The behaviour is still mostly the same (only better ;)), because
> .currentNotification implies all the other conditions previously explicit
> here. New code is faster, too :)

Fixed as described, by bug 521158 (https://hg.mozilla.org/comm-central/rev/42d26678e7e6).

Once user activates "Remind Me Later" from whereever in the UI, he will definitely get reminded via attachment reminder alert upon sending the message, regardless of circumstances.
I.e. all the guesswork of counting existing or added attachments, or re-looking at words in msg body (this bug) has been removed for that scenario where the user explicitly wants to be reminded.