Since the completion of bug 967494, the composition language preference should not change when the language is changed via the right-click menu in subject or message body. This works in the message body but not the message subject, if the message is recycled. What happens is the the eEditorMailMask (0x40) flag gets lost for the subject on a recycled message. https://dxr.mozilla.org/comm-central/source/mozilla/editor/composer/nsEditorSpellCheck.cpp#609 New message: flags = 0x143. Recycled message: flags = 0x103.

This problem was already known, see bug 967494, comment #41 and Jan's original patch (attachment 8527645 [details] [diff] [review]) where he said: + // We need to set this flag on focus event because when reusing compose window + // flags get reset by updateEditableFields when changing 'disabled' attribute. + // Reset of flags is done after javascript code is finished so there's no other + // way to set the eEditorDontUseContentPref flag during opening of reused compose window. I have to take the blame for not fixing the issue before presenting his modified patch for review :-( Back then I couldn't reproduce the problem and so I removed Jan's solution which had been rejected in the review. I'm look for another fix.

Damn those typos: I'll look for another fix I'm looking for another fix.

This is damn complicated, not easy to see where the eEditorMailMask (0x40) gets lost. Some debugging shows, that for a new composition window, when first clicking into the subject field, this gets called: nsEditor::SetFlags: 0x143 For a recycled composition window, when first clicking into the subject field, this gets called: nsEditor::SetFlags: 0x103 (eEditorMailMask missing). This occurs although the flag is set every time in InitEditor (MsgComposeCommands.js). In Jan's original patch, he simply added the eEditorMailMask every time the subject field gained focus. This approach was rejected in a review, and I foolishly removed it without any replacement.

This is a little urgent as we might want to include the fix in TB38.

I don't think this change will affect anything, but here's a try run for completeness: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cd41a67c0e4

Comment on attachment 8613328 [details] [diff] [review] Patch to preserve flags. Review of attachment 8613328 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/html/nsTextEditorState.cpp @@ +1299,5 @@ > > newEditor = mEditor; // just pretend that we have a new editor! > + > + // Don't lose application flags in the process. > + uint32_t f; Nit: please give this variable a better name, such as originalFlags. Also, please initialize it to 0.

Carrying over Ehsan's review+

Comment on attachment 8613537 [details] [diff] [review] Patch to preserve flags (nit fixed) Approval Request Comment [Feature/regressing bug #]: 967494 [User impact if declined]: This patch represents a little tweak/addition to something what was implemented in bug 967494. It would be good if it could be landed on mozilla40, so Thunderbird 40 can take advantage, since bug 967494 was landed on mozilla40 (bug 967494 comment #63). [Risks and why]: No risk for Firefox. This code is only here for Thunderbird. [String/UUID change made/needed]: None.

Comment on attachment 8613537 [details] [diff] [review] Patch to preserve flags (nit fixed) Please don't n-i the release manager without a good reason, we are scanning the uplifts. Anyway, taking it as it seems low risk.

Sorry, last time (bug 1140617, comment #53) I waited more than a week ;-( Thanks for taking it!

http://hg.mozilla.org/releases/mozilla-esr38/rev/8818cfba3036 for THUNDERBIRD_38_VERBRANCH (See bug 1170181)