Bug 7363

Summary: REGRESSION (r12872): Repro crash when clicking the Quick Reply box in Gmail
Product: WebKit Reporter: musti <mustiman>
Component: New BugsAssignee: Vicki Murley <vicki>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, alice.barraclough, darin, Graham.Dennis, mitz, vicki
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 420+   
Hardware: Macintosh   
OS: OS X 10.4   
URL: http://mail.google.com
Attachments:
Description Flags
Test case (crasher)
none
automatic testcase (caused by detached node)
none
patch none

Description musti 2006-02-19 09:16:35 PST
Steps:
Logon to GMail
Click on any message in Inbox
Click on the Quick Reply box to enter a reply
You see the input box with Check Spelling, then WebKit crashes.

Crash Report below:
Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
0   com.apple.WebCore        	0x01180b20 WebCore::DocumentImpl::setFocusNode(KXMLCore::PassRefPtr<WebCore::NodeImpl>) + 400
1   com.apple.WebCore        	0x010b0cd4 WebCore::ElementImpl::focus() + 116
2   com.apple.WebCore        	0x01064b10 KJS::HTMLElementFunction::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 1136
3   com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
4   com.apple.JavaScriptCore 	0x0012badc KJS::FunctionCallDotNode::evaluate(KJS::ExecState*) + 524
5   com.apple.JavaScriptCore 	0x00130468 KJS::ExprStatementNode::execute(KJS::ExecState*) + 104
6   com.apple.JavaScriptCore 	0x001337d8 KJS::SourceElementsNode::execute(KJS::ExecState*) + 488
7   com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
8   com.apple.JavaScriptCore 	0x0013330c KJS::TryNode::execute(KJS::ExecState*) + 108
9   com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
10  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
11  com.apple.JavaScriptCore 	0x001183f8 KJS::DeclaredFunctionImp::execute(KJS::ExecState*) + 56
12  com.apple.JavaScriptCore 	0x00117d30 KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 464
13  com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
14  com.apple.JavaScriptCore 	0x0012c27c KJS::FunctionCallResolveNode::evaluate(KJS::ExecState*) + 524
15  com.apple.JavaScriptCore 	0x00130468 KJS::ExprStatementNode::execute(KJS::ExecState*) + 104
16  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
17  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
18  com.apple.JavaScriptCore 	0x001183f8 KJS::DeclaredFunctionImp::execute(KJS::ExecState*) + 56
19  com.apple.JavaScriptCore 	0x00117d30 KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 464
20  com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
21  com.apple.JavaScriptCore 	0x0012c27c KJS::FunctionCallResolveNode::evaluate(KJS::ExecState*) + 524
22  com.apple.JavaScriptCore 	0x00130468 KJS::ExprStatementNode::execute(KJS::ExecState*) + 104
23  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
24  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
25  com.apple.JavaScriptCore 	0x00130744 KJS::IfNode::execute(KJS::ExecState*) + 484
26  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
27  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
28  com.apple.JavaScriptCore 	0x00130744 KJS::IfNode::execute(KJS::ExecState*) + 484
29  com.apple.JavaScriptCore 	0x001337d8 KJS::SourceElementsNode::execute(KJS::ExecState*) + 488
30  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
31  com.apple.JavaScriptCore 	0x0013330c KJS::TryNode::execute(KJS::ExecState*) + 108
32  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
33  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
34  com.apple.JavaScriptCore 	0x001183f8 KJS::DeclaredFunctionImp::execute(KJS::ExecState*) + 56
35  com.apple.JavaScriptCore 	0x00117d30 KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 464
36  com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
37  com.apple.WebCore        	0x010766dc KJS::ScheduledAction::execute(KJS::Window*) + 236
38  com.apple.WebCore        	0x010769d4 KJS::WindowQObject::timerFired(KJS::DOMWindowTimer*) + 148
39  com.apple.WebCore        	0x012d2164 WebCore::TimerBase::fireTimers(double, KXMLCore::Vector<WebCore::TimerBase*, (unsigned long)0> const&) + 324
40  com.apple.WebCore        	0x012d2210 WebCore::TimerBase::sharedTimerFired() + 112
41  com.apple.CoreFoundation 	0x90770aec __CFRunLoopDoTimer + 184
42  com.apple.CoreFoundation 	0x9075d464 __CFRunLoopRun + 1680
43  com.apple.CoreFoundation 	0x9075ca18 CFRunLoopRunSpecific + 268
44  com.apple.HIToolbox      	0x9318f1e0 RunCurrentEventLoopInMode + 264
45  com.apple.HIToolbox      	0x9318e874 ReceiveNextEventCommon + 380
46  com.apple.HIToolbox      	0x9318e6e0 BlockUntilNextEventMatchingListInMode + 96
47  com.apple.AppKit         	0x9366c104 _DPSNextEvent + 384
48  com.apple.AppKit         	0x9366bdc8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
49  com.apple.Safari         	0x000072d4 0x1000 + 25300
50  com.apple.AppKit         	0x9366830c -[NSApplication run] + 472
51  com.apple.AppKit         	0x93758e68 NSApplicationMain + 452
52  com.apple.Safari         	0x0005cfdc 0x1000 + 376796
53  com.apple.Safari         	0x0005ce80 0x1000 + 376448
Comment 1 mitz 2006-02-19 10:51:32 PST
Created attachment 6612 [details]
Test case (crasher)

To see the problem, open the test case and click where indicated. The second time you click, Safari will crash.
Comment 2 mitz 2006-02-19 10:56:05 PST
This is a regression from r12872 (the fix for <rdar://problem/4315673> and <rdar://problem/4447009>). The crash happens when the element losing focus no longer has a renderer. The fix may be as simple as adding a null check for r right here in DocumentImpl::setFocusNode:

        // Dispatch a change event for text fields or textareas that have been edited
        RenderObject *r = static_cast<RenderObject*>(oldFocusNode.get()->renderer());
        if ((r->isTextArea() || r->isTextField()) && r->isEdited()) {
            oldFocusNode->dispatchHTMLEvent(changeEvent, true, false);
            r->setEdited(false);
        }

However I'm not sure about the implications of not dispatching the changeEvent in this case.
Comment 3 Darin Adler 2006-02-19 15:04:39 PST
The code quoted here has another problem, too. After calling dispatchHTMLEvent, the renderer may be gone, so we can't just call r->setEdited(false) without first re-getting that renderer from the DOM node.
Comment 4 Graham Dennis 2006-02-20 23:59:30 PST
*** Bug 7398 has been marked as a duplicate of this bug. ***
Comment 5 Graham Dennis 2006-02-21 00:47:46 PST
Created attachment 6641 [details]
automatic testcase (caused by detached node)

This is a second testcase where the lack of a renderer is caused by a detached node instead of 'display: none' styling.
Comment 6 Vicki Murley 2006-02-21 14:20:12 PST
I've attached the patch that fixes this bug, which Adele reviewed and I'll commit shortly.  

The second test case here ("automatic testcase"), exhibits a second bug.  Focusing a text field from an onload handler fails, b/c the node doesn't have a renderer when onload fires.  "Automatic testcase" no longer crashes, but the text field isn't focused as it should be.  This issue has already been reported in another bug (http://bugzilla.opendarwin.org/show_bug.cgi?id=7405).  The attached patch doesn't aim to fix this second bug.
Comment 7 Vicki Murley 2006-02-21 14:23:50 PST
Created attachment 6650 [details]
patch
Comment 8 Vicki Murley 2006-02-21 14:33:29 PST
I haven't committed a test case for this yet b/c tests that use eventSender are hanging from time to time (see Justin's comment in the Changelog).  I'm seeing this hang in the test case I'm working on.
Comment 9 Alice Liu 2006-03-01 09:21:06 PST
<rdar://problem/4462712>