Last Comment Bug 762594 - Prompts to save unmodified message forward draft
: Prompts to save unmodified message forward draft
: regression
Product: Thunderbird
Classification: Client Software
Component: Message Compose Window (show other bugs)
: 13 Branch
: All All
: -- normal (vote)
: Thunderbird 20.0
Assigned To: Magnus Melin
Depends on:
Blocks: 736055
  Show dependency treegraph
Reported: 2012-06-07 11:05 PDT by Jonathan Kamens
Modified: 2012-12-30 11:52 PST (History)
7 users (show)
mkmelin+mozilla: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

proposed fix (8.72 KB, patch)
2012-11-25 02:43 PST, Magnus Melin
no flags Details | Diff | Splinter Review
proposed fix, v2 (8.70 KB, patch)
2012-11-25 02:46 PST, Magnus Melin
mconley: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑esr17+
Details | Diff | Splinter Review

Description Jonathan Kamens 2012-06-07 11:05:54 PDT
If I click the Forward button on a message, and then immediately try to close the window, TB asks me if I want to save it.

I'm fairly certain this is new behavior. If I haven't modified the draft after clicking the Forward button, then it shouldn't be prompting me, and I don't think it has in the past.

I'm marking this as a regression. Feel free to remove the keyword if I'm wrong about the previous behavior, but I don't think I am.
Comment 1 rsx11m 2012-06-07 12:13:42 PDT
TB Daily 13.0a1 build from March 3 doesn't ask on close.
TB 13.0 beta 1 (April 25) *does* ask to save as draft.

Also applies to Linux x86_64, composing in HTML mode.
Comment 2 Magnus Melin 2012-10-02 13:34:06 PDT
Regression window 

Bug 736055 looks like the most likely cause, or possibly bug 650776.
Comment 3 Mark Banner (:standard8, afk until Dec) 2012-10-24 02:08:16 PDT
Hmm, I'm pretty sure there's some sort of hack or work around to "reset" the editor when we start up the compose window, I'm hoping someone might be able to remember where that is.
Comment 4 2012-10-24 02:29:33 PDT
Well, the typical sequence goes like this (error checking removed for sanity)

/* do stuff here */

SetBodyModified(false) is a wrapper around m_editor->ResetModificationCount();
Comment 5 Magnus Melin 2012-11-24 12:31:33 PST
I've got a fix for this, just need to improve the tests for it.
Comment 6 Magnus Melin 2012-11-25 02:43:13 PST
Created attachment 684934 [details] [diff] [review]
proposed fix
Comment 7 Magnus Melin 2012-11-25 02:46:28 PST
Created attachment 684935 [details] [diff] [review]
proposed fix, v2

Sorry, forgot a qrefresh... this is the final version
Comment 8 Mike Conley (:mconley) 2012-12-07 08:21:05 PST
Comment on attachment 684935 [details] [diff] [review]
proposed fix, v2

Review of attachment 684935 [details] [diff] [review]:

I'm good with this. Thanks Magnus!
Comment 9 Magnus Melin 2012-12-09 12:39:51 PST -> FIXED
Comment 10 Magnus Melin 2012-12-09 12:42:20 PST
Comment on attachment 684935 [details] [diff] [review]
proposed fix, v2

Review of attachment 684935 [details] [diff] [review]:

Do we want this for esr?
Comment 11 Mark Banner (:standard8, afk until Dec) 2012-12-23 12:26:42 PST
Comment on attachment 684935 [details] [diff] [review]
proposed fix, v2

[Triage Comment]
Yes, I think we should take it for ESR. We'll also land on aurora ready for the next beta.
Comment 12 Mark Banner (:standard8, afk until Dec) 2012-12-23 13:41:52 PST
Comment 13 Mark Banner (:standard8, afk until Dec) 2012-12-30 11:52:21 PST

Note You need to log in before you can comment on or make changes to this bug.