Not sure whose bug this is (app bug or composer core bug) -- I may look into it a bit more to try to distinguish, but I wanted to get a bug on file about it. Following the steps to reproduce for bug 283374, I see nsEditingSession::TearDownEditorWindow called three times: 1) before nsEditingSession::PrepareForEditing is called (so it does nothing) 2) right after PrepareForEditing is called (so mDoneSetup becomes false) 3) when the compose window is closed (it does nothing, since mDoneSetup is false) Since the last one does nothing (since it remembers it's already been called), the steps in bug 283374 cause a leak of an nsTimerImpl and an nsComposerCommandsUpdater since nsComposerCommandsUpdater::NotifyDocumentWillBeDestroyed doesn't get called to null out mUpdateTimer.
The problem seems to be in nsEditingSession: the first call is from nsEditingSession::MakeWindowEditable, and the second call is from nsEditingSession::StartDocumentLoad , which seems to duplicate what the first was supposed to do, and too late.
Assignee: mscott → mozeditor
Component: Message Compose Window → Editor
Product: Thunderbird → Core
QA Contact: bugzilla
Version: unspecified → Trunk
The later call is older (revision 1.2); I think it should just be removed, it duplicates the earlier call added later (revision 1.6).
Created attachment 178645 [details] [diff] [review] patch
Assignee: mozeditor → dbaron
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 178645 [details] [diff] [review] patch Sorry, I don't know this stuff.
Attachment #178645 - Flags: review?(neil.parkwaycc.co.uk) → review?(brade)
Attachment #178645 - Flags: superreview?(roc) → superreview+
Attachment #178645 - Flags: review?(brade) → review?(sfraser_bugs)
Does this break editing pages in composer which do a charset-related reload?
Nope, that works fine. I saved a page with Japanese characters in both UTF-8 and ISO-2022-JP (as distinguished by a <meta http-equiv="content-type" content="text/html; charset=...">), and both versions loaded fine in the editor. (That's what you were asking, right?)
Does this patch work when opening/closing a frameset? Does this patch work with Midas? jst may or may not have changes that are in this same vicinity so one of you may see conflicts.
http://www.mozilla.org/editor/midasdemo/ still works fine. What do you mean by "opening/closing a frameset"?
There are many documents that contain frames / iframes. I used to always load certain pages on rottentomatoes.com to test against frames. I'm not sure if they still use frames or not. MSDN still uses frames on some pages such as: http://msdn.microsoft.com/library/default.asp?url=/workshop/author/dhtml/reference/objects.asp I would make sure that you can edit each frame. I'm actually not positive that this works in recent builds (has anyone tested it lately? Daniel?).
Also, try editing: http://www.mozilla.org/editor/midasdemo/
Comment on attachment 178645 [details] [diff] [review] patch Deny review pending more testing.
Attachment #178645 - Flags: review?(sfraser_bugs) → review-
Target Milestone: mozilla1.8beta2 → Future
I wonder if this does not impact Midas when you move from an editable page to a non-editable one through a link. Hmmmm.
You need to log in before you can comment on or make changes to this bug.