User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; ja-JP; rv:1.7.9) Gecko/20050711 Firefox/1.0.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050906 Thunderbird/1.4 ID:2005090619 On spell checking, select the correct word "hello" from context menu then crashed. Any other words that listed bellow dosen't crash. Check from toolbar button dosen't crash, and it can replace to "hello". That's very fanny! Reproducible: Always Steps to Reproduce: 1.Type "hellow" similar to "hello" in text compose area and hit enter. 2.Right click it underlined word. 3.Select the item "hello" from top of context menu. Actual Results: Thunderbird crashed. Expected Results: Replace the bad spell to "hello".
bug not reproducible in version 1.5 Beta 1 (20050908) xp home
Okay. If it's working in 1.5 then it's probably been fixed somewhere along the way. If you see it again in 1.5 try to get a talkback ID for the crash and reopen this bug.
Bug confirmed on Thunderbird 1.5 Beta 1
Okay. We need a talkback ID then. Just uninstall Thunderbird (you won't lose your e-mails) and chose to do a custom install. Then when it crashes post the talkback ID here.
Talkback ID is TB9233061H. And, I used this build on Windows 2000 SP4. /thunderbird/nightly/2005-09-10-11-mozilla1.8/thunderbird-1.4.en-US.win32.zip
Incident ID: 9233061 Stack Signature nsFontMetricsWin::ResolveForwards c11b34e7 Product ID Thunderbird15 Build ID 2005091010 Trigger Time 2005-09-10 22:59:54.0 Platform Win32 Operating System Windows NT 5.0 build 2195 Module thunderbird.exe + (000d167b) URL visited User Comments Check the spell "hellow" and select "hello" from context menu, then Thunderbird crashed. 1. Open compose window. 2. Type "hellow" in text compose area. 3. Right click it. 4. Select correct word "hello". 5. Then crashed. Since Last Crash 30 sec Total Uptime 45 sec Trigger Reason Access violation Source File, Line No. e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/gfx/src/windows/nsFontMetricsWin.cpp, line 4008 Stack Trace nsFontMetricsWin::ResolveForwards [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/gfx/src/windows/nsFontMetricsWin.cpp, line 4008] nsRenderingContextWin::GetWidth [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/gfx/src/windows/nsRenderingContextWin.cpp, line 1514] nsTextFrame::GetPointFromOffset [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/generic/nsTextFrame.cpp, line 4214] nsTypedSelection::GetPointFromOffset [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/generic/nsSelection.cpp, line 6747] nsTypedSelection::GetCachedFrameOffset [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/generic/nsSelection.cpp, line 5090] nsCaret::GetCaretRectAndInvert [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsCaret.cpp, line 988] nsCaret::DrawAtPositionWithHint [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsCaret.cpp, line 704] nsCaret::DrawCaret [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsCaret.cpp, line 908] nsCaret::StartBlinking [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsCaret.cpp, line 481] nsTypedSelection::NotifySelectionListeners [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/generic/nsSelection.cpp, line 7295] nsSelection::NotifySelectionListeners [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/generic/nsSelection.cpp, line 3007] mozInlineSpellChecker::ReplaceWord [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 391] XPTC_InvokeByIndex [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102] XPCWrappedNative::CallMethod [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2139] XPC_WN_CallMethod [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1402] js_Invoke [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1163] js_Interpret [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 3459] js_Invoke [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1183] js_InternalInvoke [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1260] JS_CallFunctionValue [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/js/src/jsapi.c, line 4017] nsJSContext::CallEventHandler [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1414] nsJSEventListener::HandleEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp, line 185] nsEventListenerManager::HandleEventSubType [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1685] nsEventListenerManager::HandleEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1786] nsXULElement::HandleDOMEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/content/xul/content/src/nsXULElement.cpp, line 2201] PresShell::HandleDOMEventWithTarget [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp, line 6477] nsMenuFrame::Execute [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 1621] nsMenuFrame::HandleEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 452] PresShell::HandleEventInternal [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp, line 6420] PresShell::HandleEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp, line 6215] nsViewManager::HandleEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/view/src/nsViewManager.cpp, line 2559] nsViewManager::DispatchEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/view/src/nsViewManager.cpp, line 2246] HandleEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/view/src/nsView.cpp, line 174] nsWindow::DispatchEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1252] nsWindow::DispatchMouseEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 5981] ChildWindow::DispatchMouseEvent [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 6232] nsWindow::WindowProc [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1434] USER32.dll + 0x2a3d0 (0x77e0a3d0) USER32.dll + 0x4605 (0x77de4605) USER32.dll + 0xa7ba (0x77dea7ba) nsAppStartup::Run [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 146] main [e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/mail/app/nsMailApp.cpp, line 62] KERNEL32.DLL + 0x2893d (0x77e7893d)
WFM, debug branch build of Thunderbird on Linux. Can someone get the regression window please? Is it branch only?
I have tested on latest trunk build with new profile. The error was occured too. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050911 Thunderbird/1.6a1 ID:2005091106
Thanks, does it occur with the oldest builds available from here? http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/
Yes, old trunk build also. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050828 Thunderbird/1.6a1 ID:2005082808
Thank you very much for your help with testing. There are even older builds available here: http://archive.mozilla.org/pub/thunderbird/nightly/ It would help to know if this also occurs in very old builds... and if it doesn't, when the crash first started to appear.
(In reply to comment #3) > Bug confirmed on Thunderbird 1.5 Beta 1 Niels, which version of Windows did you use?
I used Windows XP Home SP2 and Firefox 1.5 Beta 1
(In reply to comment #11) The error was occured since "version 0.6+ (20050203)". This is the first build that Inline Spell Checker function was checked in at Bug 278312.
This is the same problem as bug 252970. mozInlineSpellChecker::ReplaceWord() whirls into the selection which ulimately does: nsFontMetricsWin::ResolveForwards nsRenderingContextWin::GetWidth nsTextFrame::GetPointFromOffset nsTypedSelection::GetPointFromOffset [...] mozInlineSpellChecker::ReplaceWord The crash arises when there are still pending reflows. Such crashes are still in the selection code because it is the primary consumer of APIs that expect frames to have been reflowed and are clean (bug 302728). In fact, I am even amazed that there aren't many of them.
So can we fix the selection api entry points to flush reflow?
*** Bug 308181 has been marked as a duplicate of this bug. ***
*** Bug 308579 has been marked as a duplicate of this bug. ***
This is the #2 (actually #1) topcrasher for TB on 1.8 Branch and for TB 1.5 Beta 1.
Created attachment 196597 [details] [diff] [review] never crash I propose experimenting this patch since we are still early in 1.9. (Add yourself to the #ifdef if your are interested in getting the warnings.) Rationale for the patch: - Better fail than crash. - See it as null checks--you can never be too sure of calls beyond your control. - Those who get there should not have gone there in the first place. I noted that most of the calls arise because of selection.Collapse(), which is why these out-of-sync issues have not been too damaging so far (in terms of the user experience) even when the crashes do not happen. - It is messy (and costly) to flush reflows all over the place in the selection.
Created attachment 196599 [details] [diff] [review] never crash take this one instead... I clicked the wrong file in my previous attachment.
Created attachment 196687 [details] [diff] [review] simpler Take 3. Simplify the |if| check because first-reflow implies dirty. I was using NS_ASSERTION instead of NS_WARNING and the !isZero test only helps to cut the number of assertions/warnings.
Comment on attachment 196599 [details] [diff] [review] never crash I am off for a trip until the end of the week. If you r/sr before then, please follow-up with the check-in (and the the unexpected...).
I am back and can follow-up in the next two weeks (after which I will be travelling again).
I'm going to try to get to this in the next day or two...
Comment on attachment 196687 [details] [diff] [review] simpler So... For trunk this looks ok, I guess, but I'd like a followup on documenting which methods expect layout to be flushed, seeing asserts to that effect added in general, and then fixing callers to flush as needed. Or addressing this in some other way, if necessary... For branch, this looks pretty scary, but if we're hitting these crashes a lot maybe it's worth trying after some trunk bake time.
Checked in the trunk. Will wait for it to bake on the trunk a bit before asking for approval on the branch. I don't feel the patch is too scary (compared to those crashes that are _guaranted_ to be there otherwise). It is common sense to me. In fact, I feel this defense (with a hard assertion instead of a mere warning) should have been there in the very early days to assist the development. People have since piled things up without enforcing the constraint and/or realising the consequences. Bug 302728 is meant for the follow-up.
Is it possible this caused bug 310227?
This might have also caused bug 310318.
If you cannot fix bug 310318 soon, please back out this patch. We cannot input Japanese text. This is very serious problem.
Only two bugs so far? Not too bad... I was rubbing my hands for nastier things... I will take a look at them. Perhaps we might be able to secure the much bigger reward of this fix after all. Masayuki, simply change the NS_WARNING to NS_ASSERTION, and add yourself to the #ifdef. This way, you will quickly see the precise spot where to flush the reflow. The assertion might also give you another nice and lightweight idea to resolve the IME matter. Of course, the backout is there if the issues can't be resolved soon enough.
RBS and Masayuki, time is running out for 1.5 and we need traction on those regressions before we can consider this for the branch. Can you give us an update here today?
I have checked a fix for the two known regressions so far. I simply disabled parts of the patch. Fortunately, the parts that are still active are the parts that protect against the reported crashes. We still need a few days to ascertain if things have settled.
rbs, can you post a branch only patch that backs out the changes you no longer need so we can get a look at what exactly we need to change on the branch?
Created attachment 198363 [details] [diff] [review] patch for the branch Here is a branch only patch without the changes that caused the regressions. Note that althougth this is a topcrasher, I can't help be a bit nervous about this patch because it hasn't got enough baking on the trunk yet. Unfortunately, the beta is going to take the focus and nobody is going to bother to download/test the trunk anymore when the beta is out.
peterv, roc, dbaron, bz, what do you guys think about taking this change without much opportunity for testing before we ship beta (maybe one day of builds.)
Pretty much what I said in comment 26. It's pretty scary to me, but the thing that really scared me (things that would consistently fail because the always happen when the frame is dirty) is what would have gotten caught first (and some did). So it might be ok if people test the heck out of everything selection-related during that one day. Especially on pages with animated stuff (marquee, tickers, that sort of thing). Past beta 1, by the way, I think we should make these real assertions on the trunk (I know that sucks on windows but there is no way anyone will notice warnings -- I sure don't in the mass of debug spew I get from debug builds) and start fixing callers (trunk and possibly branch, depending on the caller and the patch) so that we can make sure issues really don't arise.
If it helps you -- I had applied the "simpler" patch to my self-built Thunderbird 1.5b1, and I'm using it since Sept. 22 for my daily mail. While TB 1.5b1 without that patch was almost unusable when replying to emails, the patched version is much more stable.
we're going to take this for the Release Candidate right after the branch re-opens for post beta 2 work. The risk was just too high to take it without any time to test it.
This had also caused another regression: Highlighting text via keyboard and then using Ctrl-X and Ctrl-V to move it made the selection all weird. The backout that fixed the other regressions helped those bugs too...
we'll try for RC1.
Comment on attachment 198363 [details] [diff] [review] patch for the branch moving request to 1.8rc1 since 1.8b5 has shipped.
As per my comment 24, I am off again for a trip and will be away for a week. Feel free to check in the patch at the opportune time.
ok, this patch for the branch is now checked in. Marking this closed.