950914 - [Messages][Drafts] Replace existing draft when hiding app while showing unsent, existing draft content

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Evelyn Eastmond [:evhan55]]

STR:

1) Create a draft to recipient '333' and save
2) Click back to go to thread list
3) Click back into the same thread
4) Hide the app with home button press
5) Launch messages app
6) Hide the app with home button press
7) Launch messages app
8) Click back to go to thread list, choose 'Save as Draft'

Expected:
- There is only one draft entry to recipient '333'
- The draft was simply replaced

Actual:
- There are multiple draft entries to recipient '333'
- New drafts were made every time the app was hidden

Comment 1

[Evelyn Eastmond [:evhan55]]

This may not be trivial to solve given the current spec and the current code.
I'd like to run something by you Rick:

*Brand New Draft*
When you hide and recall the app, new drafts get saved every time, because it's an implicit save without a dialog.  We discussed this is to spec, and we implemented it that way against our recommendations.

*Existing Draft*
When you hide and recall the app *the first time*, it does correctly replace, actually.  But then, MessageManager.draft is set to null, correctly, since the draft is considered saved and done with.  Now, when you hide and recall the app *the following times*, it now considers the stuff in the composer to all be a fresh new draft, and it starts saving new drafts, just like in the case above.

So, this bug (950914) is actually not a bug, the replacement does indeed happen the first time the app is hidden/recalled*.  But, hiding and recalling more times creates new drafts, which is to spec.

Do you think this is worth solving or should we leave it as another quirk of the current spec?

* This was tested on rwaldron/v1.3-drafts with rwaldron/947211 applied on top

Comment 2

[Evelyn Eastmond [:evhan55]]

From IRC

<rwaldron> you could just make saveMessageDraft have two paths
<rwaldron> onVisibilityChange => don't clear MessageManager.draft
<rwaldron> Everythings else => clear it
<evhan55> okay! that would be easy
<rwaldron> ThreadUI.saveMessageDraft({ preserve: true });
<rwaldron> or something
<rwaldron> then at the end of ThreadUI.saveMessageDraft... if !opts.preserve...
<rwaldron> seems ok

Comment 3

[Evelyn Eastmond [:evhan55]]

Attachment: https://github.com/rwaldron/gaia/pull/12

- `message_manager.js`
    - specify that the draft should be preserved when app hides
- `thread_ui.js`
    - if draft is specified to be preserved in `saveDraft`, preserve it

Tests
- `message_manager_test.js`
    - make sure preserve flag is passed when app hides
- `thread_ui_test.js`
    - make sure draft is preserved (or not) depending on what is specified in `saveDraft`

Comment 4

[Rick Waldron [:rwaldron]]

Landed on v1.3-drafts https://github.com/rwaldron/gaia/commit/00f9e5b25276cbcbff6e0be8f31a2767f2a2a81d

Comment 5

[Rick Waldron [:rwaldron]]

https://github.com/mozilla-b2g/gaia/commit/41377d094fdc4dd8630d386e04d807938d1d2fce

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2477e5a5c9f1