Edward Chen[:edchen] Reporter
	Description • 10 years ago

[Environment]
Gaia      2402076e6299ab36f492eab17795478c9d2a7ad7
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/c41d7012974e
BuildID   20140617160202
Version   32.0a2
ro.build.version.incremental=94
ro.build.date=Tue May 20 09:29:20 CST 2014

[STR]
1. Launch email app
2. Set a mail account
3. Write a mail 
4. Save the mail into local drafts
5. Go to local drafts
6. Edit prior mail
7. Tap "<", user does not save it 
8. Tap discard
9. The mail will be deleted.

[Actual result]
1. The mail will be deleted.

[Expected result]
1. The mail does not be deleted.
2. The mail content will recover to prior content.

	robert.sajdok Assignee
	Comment 1 • 10 years ago

I checked similar user case in gmail and everything is ok.

	Marcus Cavanaugh [:mcav]
	Comment 2 • 10 years ago

Are you expecting the mail to end up in your trash folder? "Discard", in this case, is used to mean "Delete".

	bhavana bajaj [:bajaj]
	Comment 3 • 10 years ago

Is this not going to trash or being permanently deleted which means the trash is not working correctly ? Can you add this information ?

	Edward Chen[:edchen] Reporter
	Comment 4 • 10 years ago

1. Actually, I expected this case should discard user edited content. and orignal mail does not be deleted. 
2. Even this draft deleted to end up in trash, but we cannot see the mail in trash.

	Dylan Oliver [:doliver]
	Comment 6 • 10 years ago

Clearing blocking flag as this is not a regression and is likely working as expected. Reasonable minds could disagree about the "correct" behavior here but I think any change is more of a feature request than a bug.

(In reply to Edward Chen[:edchen] from comment #4)
> 2. Even this draft deleted to end up in trash, but we cannot see the mail in
> trash.

This does not show up in Trash because Trash is a view of the folder on the server and as this was a local draft, it was never sent to the server. We'd need a "Local Trash" to store it in instead.

	James Burke [:jrburke]
	Comment 7 • 10 years ago

Comment on attachment 8444428 [details] [review]
Proposed fix.

Clearing review for now since we likely need UX input to work out the final fix for this. As doliver mentions, this is a bit of a change in the UX, and as far as I know, this has been the draft behavior for a while.

On the specific change in the pull request, the pull request change would lead to slightly inconsistent behavior: the email app will save the draft automatically if the email app goes into the background. For instance, when selecting the + icon to add a recipient from the Contacts app, or to get an attachment. So by not doing the abortCompositionDeleteDraft(), it would result in drafts the user may not want to keep around in the first place:

If they start a fresh draft, add a contact, then decide they don't want to send an email, the "Discard" action when going back to the message_list would not really be a Discard then, because the autosaved draft would show up in Local Drafts.

For the general bug UX question, flagging Juwei to see if UX would like a different behavior for Discard from Local Draft navigation vs Discard from first Compose navigation. However, note that while UX can specify a different behavior than what is done now, a technical fix would likely be a bit of work, such that it would not qualify for 2.0 inclusion.

If Juwei indicates UX wants the same Discard behavior for both cases, then we should close this bug.

	Juwei Huang[:juwei]
	Comment 8 • 10 years ago

I can see the confusion here is more about the wording "Discard".
If the behaviour is to delete the local draft when tap on back button, the wording should states "Delete the draft"

Yet there is also a situation is that users want to discard the changes only and keep the draft on the folder. In this case we didn't provide any way to do that now.

I suggest that for the version one we could change the wording first. And for version two we could add one more action "Discard Changes" so that users still can keep the draft without changes.

So the flow could be:

1. Open a message on local draft folder
2.1 If users tap on back button without changing anything, the screen will go back directly to local draft folder.
2.2 If users change the content and tap on back button, an action menu pops up with 3 options:
 - "Discard Changes" : discard the changes and save the draft
 - "Save to Local Drafts": save the changes with the draft
 - "Delete the draft": delete the draft directly

Let me know your thought :)

	Andrew Sutherland [:asuth] (he/him)
	Comment 9 • 10 years ago

Providing "revert to last explicitly saved draft" is a bit of a logistical nightmare, especially in the face of autosave, and especially when we start saving drafts to the server (which I understand to still be an explicit goal).

In fact, I'm not sure there are any email apps out there that provide the "revert to last saved" functionality.  Is anyone aware of any?

(I did test to see if gmail provided some explicit revert affordance or transparently persisted its undo stack, but neither seems to occur.)

In any event, I'm all on board for clarifying that we will irrevocably delete the draft, but "discard changes" is extremely complex and I'd argue not worth the implementation effort, let alone the maintenance burden.

	James Burke [:jrburke]
	Comment 10 • 10 years ago

Sounds like the minimum fix (and likely the only change) the text to be "Delete the draft" from "Discard" when coming from the Local Drafts folder. Given that we are at string freeze for 2.0 and this has been how email has shipped for a few releases, this should be something that is addressed in 2.1.

	Andrew Sutherland [:asuth] (he/him)
	Comment 11 • 10 years ago

While cleaning up wontfixes (for purposes along these lines) I found bug 963229 which was also asking for transactional draft handling and which we/:arog aggressively WONTFIXed.  Since I think my comment 9 is also basically arguing for WONTFIX on that, I'm changing the subject of this bug to only be about the string change.

	Joshua Mitchell (Inactive)
	Comment 12 • 9 years ago

This issue seems to have fallen off the map but is still occuring in Flame KK 3.0

This issue occurs in both Email and Messages

I agree with comment 8 about the best possible flow for the UI but given comment 9 describing the scope of adding a 'discard changes' function it seems that it might be asking for too much
HOWEVER, The current usage of just the word "Discard" IS confusing as to which type of discard is happening (the entire draft or just the most recent changes).

We should at least address the ambiguity in this language with changing it to "Discard Draft" or even "Discard Entire Draft" or similar as stated in comment 10

Device: Flame 3.0 (KK - Nightly - Full Flash - 319mem)
Build ID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0


Device: Flame 2.2 (KK - Nightly - Full Flash - 319mem)
Build ID: 20150408002503
Gaia: ea735c21bfb0d78333213ff0376fce1eac89ead6
Gecko: 43041c78052b
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

	James Burke [:jrburke]
	Comment 14 • 9 years ago

Comment on attachment 8590949 [details] [review]
[gaia] jrburke:bug1026967-email-delete-draft > mozilla-b2g:master

Changed the data-l10n-id to a new ID to indicate we want to use stronger language around deletion, the meaning of the string has been "upgraded" to a more serious result.

The string is now "Delete the draft".

While this should be easy to land on master, my understanding is that 2.2 is at string freeze already so I will not be asking for 2.2 approval, just fixing on master.

	GitHub Autolander Bot
	Comment 15 • 9 years ago

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/c8cb0c0ebb8dd1f5c0c9037e38f8e4b237beb77b

	Shally Li（leave from Mozilla）
	Comment 16 • 9 years ago

This bug has been verified as pass on latest Nightly build of Flame v3.0 and Nexus 5 v3.0 by the STR in Comment 0.

Actual results: Tapping "<" icon after creating a new mail or changing the draft in Email, it shows "Delete the draft" as expected.
See attachment: verified_v3.0.png
Reproduce rate: 0/6


Device: Flame v3.0 build(Pass)
Build ID               20150601160204
Gaia Revision          6d477a7884273886605049b20f60af5c1583a150
Gaia Date              2015-06-01 16:41:42
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/56241c1f8a3b
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150601.193935
Firmware Date          Mon Jun  1 19:39:44 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 v3.0 build(Pass)
Build ID               20150601075320
Gaia Revision          85e6fcef45c0cb2c017739df42b68b96cf5bb9c3
Gaia Date              2015-06-01 06:40:19
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/666b584fb521
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150601.112653
Firmware Date          Mon Jun  1 11:27:09 EDT 2015
Bootloader             HHZ12f