959196 - [Messages][Drafts] Don't auto-save new draft when hiding app and then draft is discarded

Status: RESOLVED FIXED

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

Description

[Evelyn Eastmond [:evhan55]]

The spec originally asked for this behavior, but the behavior is being altered.
This bug was originally reported in bugs 952514 and 958103:
https://bugzilla.mozilla.org/show_bug.cgi?id=958103#c10

STR:
* open "new message" view
* add some text in the composer
* press home
* press the "messages" icon
* press back
* press "discard draft"

Expected:
* no new draft

Actual:
* a new draft was created

Comment 1

[Julien Wajsberg [:julienw]]

I think the draft that was created should just be discarded when we get a visibilitychange event with document.hidden=false. Don't know how easy it is though ;)

Comment 2

[Evelyn Eastmond [:evhan55]]

(In reply to Julien Wajsberg [:julienw] from comment #1)
> I think the draft that was created should just be discarded when we get a
> visibilitychange event with document.hidden=false. Don't know how easy it is
> though ;)

The problem was happening because MessageManager.draft was not properly set if this brand new draft was auto-saved.  Fixing that issue fixes this bug, so I think that may be a better fix in my opinion.  I will feedback? you in any case.  Thank you!

Comment 3

[Evelyn Eastmond [:evhan55]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/15273

- `thread_ui.js`
    - If `MessageManager` is not already holding onto a draft when a brand new is auto-saved, then set it. this way preservation/discarding works as expected
- Tests
    - `thread_ui_test.js`
        - Test that `MessageManager.draft` is properly set if it didn't exist before saving a draft

Comment 4

[Rick Waldron [:rwaldron]]

Comment on attachment 8359428 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/15273

Nice r=me

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8359428 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/15273

good for me too.

2 questions/comments for you (and for me ;) ) though
* Can you please clarify what "preserve" means exactly ?
* I would have added a "visibilitychange" handler in ThreadUI instead of using the one in MessageManager. Also, why calling Drafts.store() at the end even when we don't save drafts?

Comment 7

[Evelyn Eastmond [:evhan55]]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Comment on attachment 8359428 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/15273
> 
> good for me too.
> 
> 2 questions/comments for you (and for me ;) ) though
> * Can you please clarify what "preserve" means exactly ?

Addressed in patch with a clearer comment.

> * I would have added a "visibilitychange" handler in ThreadUI instead of
> using the one in MessageManager. Also, why calling Drafts.store() at the end
> even when we don't save drafts?

Reported as a new bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=959696

Comment 8

[Evelyn Eastmond [:evhan55]]

Closed and landed: https://github.com/mozilla-b2g/gaia/commit/08e03dc347187d69e1ca7223e1eb1c368844e6cb