1850192 - changing identity of draft being composed doesn't mark the draft as modified

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Jonathan Kamens]

1. Create, save, and close a draft.
2. Open the draft for editing.
3. Change the identity that the draft is to be sent with.
4. Close the compose window.
5. Note that it doesn't prompt you to save the modified message.

Changing the identity modifies the message and therefore you should be prompted to save when you go to close the window.

Comment 1

[Anje]

I can also produce this testing with beta 118.0b1

Modify only From address
1. Create, save, and close a draft.
2. Open the draft for editing.
3. Change the identity that the draft is to be sent with - using 'customise from address'
4. Close the compose window.
5. Note that it does not prompt you to save the modified message.
6. It does not Save draft.

repeat the above for two additional tests:
Modify From address AND content

1. Create, save, and close a draft.
2. Open the draft for editing.
3. Change the identity that the draft is to be sent with - using 'customise from address'
4. Edit/Modify the content.
5. Close the compose window.
6. Note that it does prompt you to save the modified message.
7. It does Save draft if 'saved'

Modify From address and Subject

1. Create, save, and close a draft.
2. Open the draft for editing.
3. Change the identity that the draft is to be sent with - using 'customise from address'
4. Edit/Modify the Subject
5. Close the compose window.
6. Note that it does prompt you to save the modified message.
7. It does Save draft if 'saved'

Only fails to prompt to save if only the From is modified.

Comment 2

[Hartmut Welpmann [:welpy-cw]]

Attachment: Bug 1850192 - Set draft as modified when changing identity. r=#thunderbird-reviewers

Comment 3

[Hartmut Welpmann [:welpy-cw]]

Attachment: Bug 1850192 - Set draft as modified when changing identity. r=#thunderbird-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f7a2ec3991ab
Set draft as modified when changing identity. r=vineet

Comment 5

[Axel Grude]

I think this is exactly wrong behavior. if isModified = false it means the user has not entered any data. Changing the from address should not set this flag. there are add-ons (such as identity picker) who force picking an identity. This means now the user will always be prompted to save if they want to just close the compose window. It also has ramifications to my Add-on SmartTemplates, which rebuilds the email with account specific HTML and needs to know whether user inputed data (e.g. typed text) has been entered and needs to guard against data loss.

This basically perverts the "body modification" flag which was explictely designed for having changed something in the body of the mail (not any headers). I suggest the change is either rolled back or put behind an option so users can disable this problematic behavior.

See also https://bugzilla.mozilla.org/show_bug.cgi?id=2014004

Comment 6

[Jonathan Kamens]

To my mind, the purpose of the flag is clearly to indicate, "This draft needs to be saved before the window is closed; otherwise information will be lost and when the draft is reopened it will not be the same as it was before the window was closed.

The sender identity clearly falls into that category, since sender identity information is saved in the header of the draft. If the user changes the sender identity, then closes the window without saving, then when they reopen the window the identity will have reverted, i.e., their change will have been lost.

In short, changing the sender identity is "entering data" and should be reflected as such in the modified flag.

This basically perverts the "body modification" flag

Or maybe the flag is just misnamed and should be called "message modified," which I believe was/is its actual intent.

Changing the from address should not set this flag. there are add-ons (such as identity picker) who force picking an identity. This means now the user will always be prompted to save if they want to just close the compose window.

If identity picker does not consider an identity chosen through it to be a change requiring the draft to be saved, then it can capture the value of the flag immediately before it makes its change, then restore it after making the change. This would be preferable to data being lost when people who don't use identity picker change the sender identity and then close the window.

It also has ramifications to my Add-on SmartTemplates, which rebuilds the email with account specific HTML and needs to know whether user inputed data (e.g. typed text) has been entered and needs to guard against data loss.

I don't really understand exactly what you're getting at here so I can't comment directly on whether there are other workarounds for the problem you're describing, but I will say that allowing the user to close the draft window without prompting for save after the user has changed the sender identity is ALSO data loss, so you're asking the developers to make it easy for your add-on to protect against data loss by enabling another form of data loss in core Thunderbird. This is not, to my mind, an acceptable trade-off.

Maybe you can make a case for there being a separate flag specifically indicating body modification, but I don't think you can make a case for reverting this change entirely.

Comment 7

[Axel Grude]

(In reply to Jonathan Kamens from comment #6)

To my mind, the purpose of the flag is clearly to indicate, "This draft needs to be saved before the window is closed; otherwise information will be lost and when the draft is reopened it will not be the same as it was before the window was closed.

It's not called emailModified, it's clearly bodyModified - along with the modification count it is there to monitor editing within the email. so let's agree to disagree on this one.

The sender identity clearly falls into that category, since sender identity information is saved in the header of the draft. If the user changes the sender identity, then closes the window without saving, then when they reopen the window the identity will have reverted, i.e., their change will have been lost.

it also happens when they select the same identity (identity picker forces them to do this as a safety measure). So the body becomes permanently "modified" and they will always be asked to save a draft - additional, unnecessary workload for the user. The best workaround would be to expose this (in my mind FAULTY) behavior in a Thunderbird setting.

In short, changing the sender identity is "entering data" and should be reflected as such in the modified flag.

no I disagree strongly - that is not data. Entering a paragraph with many images and potentially having spent 60 minutes of work is. You haven't started editing and potentially entered many paragraphs of text - that's what the draft warning is for. If you open an email and simply select a different server - there is nothing to be gained to create a "empty" draft. Why not let the user change their mind quickly and close the composer?

This basically perverts the "body modification" flag

Or maybe the flag is just misnamed and should be called "message modified," which I believe was/is its actual intent.

If identity picker does not consider an identity chosen through it to be a change requiring the draft to be saved, then it can capture the value of the flag immediately before it makes its change, then restore it after making the change.

No. The problem - they change the identity and then Thunderbird first sets isBodyModifed and immediately emits the compose-from-changed event. This is specifically designed to code reactions once the address changes (for example doing something to the signature etc) - and SmartTemplates reacts by loading the account specific template (it rebuilds the email) - at this stage it needs to know whether the user already entered any data, to avoid loss. That's why I add a safety confirmation dialog in this case.

What needs to happen - if an extension decides to change the "from" address they must be able to keep isBodyModifed = false (the API has a parameter for this) - but this is ignored and set to true anyway - I tested this by patching identity picker. That's the main problem. If they call the API afterwards again to reset it to false, the compose-from-changed event was already fired, so it's too late at that stage.

Or maybe the flag is just misnamed and should be called "message modified," which I believe was/is its actual intent.
Maybe you can make a case for there being a separate flag specifically indicating body modification, but I don't think you can make a case for reverting this change entirely.

And what would you call that? isBodyReallyModified?

Clearly it was named correclty and this change broke it by generalizing / broadening the behavior. It creates extra work for my users! And I cannot work around it without completely reimplementing my own isBodyModified functionality somehow

Comment 8

[Anje]

Update on what occurs in Version 140.7.1esr

If I open a new Write message which auto has my signature but no other content and only change the From and then click on X to close:
If I open a new Write message which auto has my signature but no other content and only enter a subject and then click on X to close:
If I open any saved draft with content and only change the From and then click on X to close:
If I open any saved draft and alter anything - from, subject or content:

In all situations - I am prompted - save, discard or cancel.

Do I expect to see that prompt?
Yes. Because I consider anything that has been altered, added, removed etc etc in all areas of the email to be part of the data that needs to be saved, otherwise I would not have made any changes at all.

Maybe the 'isBodyModified' is not the correct term as some say the 'body' of an email excludes what I'll call header information eg: From, To, Subject, and therefore 'isModified' is more correct. I consider adding or removing an attachment to be part of 'body'.
Or create two separate sections of code - 'isBodyModified' and 'isHdrsModified' and if either triggers a true then offer the prompt save, discard or cancel.

Whatever, you choose to call it, please do not alter what it does because it works perfectly and as expected because to me if anything is modified, I expect a prompt to save, discard or cancel if I choose to exit.

Comment 9

[Axel Grude]

(In reply to Anje from comment #8)

Update on what occurs in Version 140.7.1esr

If I open a new Write message which auto has my signature but no other content and only change the From and then click on X to close:
If I open a new Write message which auto has my signature but no other content and only enter a subject and then click on X to close:
If I open any saved draft with content and only change the From and then click on X to close:
If I open any saved draft and alter anything - from, subject or content:

In all situations - I am prompted - save, discard or cancel.

Do I expect to see that prompt?
Yes. Because I consider anything that has been altered, added, removed etc etc in all areas of the email to be part of the data that needs to be saved, otherwise I would not have made any changes at all.

I implemented a workaround for my templating solution (SmartTemplates) to simulate a "body modification" in case Thunderbird sports bodyModified: I add a hash (basically a checksum) of the main container contents and save it as an attribute. Then, when user changes the "from" or something else to cause setting the isModified=true flag, I generate a new checksum and check it for changes. If it is the same, I ignore the flag now and assume the user didn't add any edited changes, and then I can swap out the template for the new account template without having to prompt them.

The idea is that it automatically loads the new "stationery" as long as the user hasn't typed anything. The old template can safely be discarded and the new one swapped in. Otherwise (isModified==true and checksum has changed), I have to ask them a combination question:

  "st.notification.bodyModified": {
    "message": "Thunderbird says you have already typed something, do you want to load the template anyway?"
  },
  "st.notification.bodyModified.accountTemplate": {
    "message": "This will discard anything  typed and load the account template."
  },
  "st.notification.bodyModified.externalTemplate": {
    "message": "This will discard anything  typed and reload the current template [$template$].",
    "placeholders": {
      "template": {
        "content": "$1"
      }
    }
  },

if they then press Cancel I won't modify anything in the email to avoid data loss.

Good thing now: I do not rely on the original behavior of Thunderbird checking edits in the body of the email. That should be safe enough to avoid data loss without bombarding my users with unnecessary, anxiety inducing questions every time they change the identity or any other stuff in the email. We are only interested in the actual content of the editor, because that's where we spend all of our time.

See also:
https://github.com/RealRaven2000/SmartTemplates/issues/415#issuecomment-3848331529