Closed Bug 1169996 Opened 7 years ago Closed 7 years ago
Changing the spell check language in the message subject of a recycled message via right-click changes the composition language preference
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.
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.
Assignee: nobody → mozilla
Component: Message Compose Window → Editor
Product: Thunderbird → Core
This is a little urgent as we might want to include the fix in TB38.
Attachment #8613328 - Flags: review?(ehsan)
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.
Attachment #8613328 - Flags: review?(ehsan) → review+
Carrying over Ehsan's review+
Status: NEW → ASSIGNED
OS: Unspecified → All
Hardware: Unspecified → All
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.
Attachment #8613537 - Flags: approval-mozilla-aurora?
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.
Attachment #8613537 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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)
You need to log in before you can comment on or make changes to this bug.