Closed Bug 637975 Opened 10 years ago Closed 9 years ago

Intermittent failure in editor/libeditor/text/tests/test_bug636465.xul | Setting the spellcheck attribute back to true should work

Categories

(Core :: DOM: Editor, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla5
Tracking Status
blocking2.0 --- -
status2.0 --- wontfix

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

(Keywords: intermittent-failure, Whiteboard: [see comment 257])

Attachments

(1 file)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1299031932.1299033500.13321.gz
Rev3 WINNT 5.1 mozilla-central opt test mochitest-other on 2011/03/01 18:12:12

7216 INFO TEST-START | chrome://mochitests/content/chrome/editor/libeditor/text/tests/test_bug636465.xul
7217 INFO TEST-PASS | chrome://mochitests/content/chrome/editor/libeditor/text/tests/test_bug636465.xul | Setting the spellcheck attribute to false should work
7218 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/editor/libeditor/text/tests/test_bug636465.xul | Setting the spellcheck attribute back to true should work
7219 INFO TEST-PASS | chrome://mochitests/content/chrome/editor/libeditor/text/tests/test_bug636465.xul | Unsetting the spellcheck attribute should work
7220 INFO TEST-END | chrome://mochitests/content/chrome/editor/libeditor/text/tests/test_bug636465.xul | finished in 110ms
I investigated this, and it actually seems like a code problem.  The restyling caused by setting the spellcheck attribute may be flushed in the middle of mozInlineSpellChecker::DoSpellCheck, which causes the textcontrol to be reframed, and the editor being reattached to the content, under this stack:

nsEditor::PreDestroy(int)+0x0000005B [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x00B18A95]
nsTextEditorState::DestroyEditor()+0x000000A1 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x007D862B]
nsTextEditorState::UnbindFromFrame(nsTextControlFrame*)+0x00000234 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x007DD8E4]
nsHTMLInputElement::UnbindFromFrame(nsTextControlFrame*)+0x00000038 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x008130BC]
nsTextControlFrame::DestroyFrom(nsIFrame*)+0x00000093 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x003AD199]
nsFrameList::DestroyFramesFrom(nsIFrame*)+0x00000068 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x00406706]
nsContainerFrame::DestroyFrom(nsIFrame*)+0x00000049 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x003E6255]
nsBoxFrame::DestroyFrom(nsIFrame*)+0x00000046 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x005B1BD4]
nsIFrame::Destroy()+0x00000024 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002FDEEA]
nsBoxFrame::RemoveFrame(nsIAtom*, nsIFrame*)+0x000000C4 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x005B119E]
nsFrameManager::RemoveFrame(nsIAtom*, nsIFrame*)+0x00000129 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x0034751B]
nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags, int*)+0x000007E1 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002FA6DB]
nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, int)+0x0000034D [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002FAC71]
nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList&)+0x000001DC [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002FB768]
nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, int)+0x000001C0 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002FBF2C]
mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint)+0x0000013C [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002E24BC]
mozilla::css::RestyleTracker::ProcessRestyles()+0x000003AF [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002E104F]
nsCSSFrameConstructor::ProcessPendingRestyles()+0x000000D6 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x002FBCBE]
PresShell::FlushPendingNotifications(mozFlushType)+0x000002E1 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x00382641]
nsDocument::FlushPendingNotifications(mozFlushType)+0x0000021C [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x00655E58]
nsComputedDOMStyle::GetPropertyCSSValue(nsAString_internal const&, nsIDOMCSSValue**)+0x000001C6 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x0050563E]
nsComputedDOMStyle::GetPropertyValue(nsAString_internal const&, nsAString_internal&)+0x0000005D [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x0050502F]
IsBreakElement(nsIDOMViewCSS*, nsIDOMNode*)+0x00000149 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014E2FA9]
mozInlineSpellWordUtil::BuildSoftText()+0x000000C2 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014E334C]
mozInlineSpellWordUtil::EnsureWords()+0x00000021 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014E367F]
mozInlineSpellWordUtil::SetPosition(nsIDOMNode*, int)+0x0000007C [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014E3C2E]
mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil&, nsISelection*, mozInlineSpellStatus*, int*)+0x0000052D [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014DD9C7]
mozInlineSpellChecker::ResumeCheck(mozInlineSpellStatus*)+0x000002E4 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014E0192]
mozInlineSpellResume::Run()+0x0000002D [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x014E188B]
nsThread::ProcessNextEvent(int, int*)+0x0000038A [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x01719E84]
NS_ProcessPendingEvents_P(nsIThread*, unsigned int)+0x00000087 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x0169FEE7]
nsBaseAppShell::NativeEventCallback()+0x000000B8 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x01413448]
nsAppShell::ProcessGeckoEvents(void*)+0x00000150 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x013C626A]
__CFRunLoopDoSources0+0x00000551 [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0004E401]
__CFRunLoopRun+0x00000369 [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0004C5F9]
CFRunLoopRunSpecific+0x0000023F [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation +0x0004BDBF]
RunCurrentEventLoopInMode+0x0000014D [/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x0002E93A]
ReceiveNextEventCommon+0x00000094 [/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x0002E69D]
BlockUntilNextEventMatchingListInMode+0x0000003B [/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox +0x0002E5F8]
_DPSNextEvent+0x000002CE [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x00043E64]
-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]+0x0000009B [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x000437A9]
-[NSApplication run]+0x0000018B [/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit +0x0000948B]
nsAppShell::Run()+0x0000007A [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x013C5B86]
nsAppStartup::Run()+0x0000008C [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x01115F14]
XRE_main+0x00002B19 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/bin/XUL +0x0002E94A]
main+0x000002F3 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/MinefieldDebug.app/Contents/MacOS/firefox-bin +0x00001287]

This can effectively cause setting the spellcheck attribute to true to have no effect.
Attached patch Patch (v1)Splinter Review
After giving this quite a bit of thought, and trying many different things, here is my take on this.

There is no way to completely address the issues in the existing code.  From the Gecko side, the editor expects the spellcheck attribute to be used in order to set up its spell checker reliably, and from the textbox.xml side, the only way that the current setup can work is for the textbox binding to know when reframes happen, which is clearly not desired.

So, let's keep things sane, and instead of trying to attach the spell checker using a duct tape to the editor from XBL, just let the spellcheck attribute inherit on the input element, so that everything would just work.

This fixes the orange quite reliably for me locally.
Attachment #519280 - Flags: review?(roc)
Depends on: post2.0
http://hg.mozilla.org/projects/cedar/rev/8e51d8fc07f4
Whiteboard: [orange][post-2.0] → [orange][post-2.0][fixed-in-cedar]
http://hg.mozilla.org/mozilla-central/rev/24b7bb6ef692
Status: NEW → RESOLVED
Closed: 9 years ago
No longer depends on: post2.0
Resolution: --- → FIXED
Whiteboard: [orange][post-2.0][fixed-in-cedar] → [orange]
Target Milestone: --- → mozilla2.2
blocking2.0: --- → ?
Even though this is an annoying orange, the fix changes the code, and it's not safe for branch.  Sorry.
blocking2.0: ? → -
Whiteboard: [orange] → [orange][see comment 257]
(In reply to comment #257)
> Even though this is an annoying orange, the fix changes the code, and it's not
> safe for branch.  Sorry.

Can't a workaround (= a conditional todo() for example) be added to m-2.0 test?
(In reply to comment #260)
> (In reply to comment #257)
> > Even though this is an annoying orange, the fix changes the code, and it's not
> > safe for branch.  Sorry.
> 
> Can't a workaround (= a conditional todo() for example) be added to m-2.0 test?

The only workaround that I can think of is just dropping the test, which doesn't sound appealing.