Bug 9911

Summary: REGRESSION: Reproducible crash on FCKeditor demo
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: DOMAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Critical Keywords: InRadar, NeedsReduction, Regression
Priority: P1    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.fckeditor.net/_temp/test_safari.html
Bug Depends on:    
Bug Blocks: 9915    
Attachments:
Description Flags
Stack trace from crash
none
patch with detailed change log and a layout test none

David Kilzer (:ddkilzer)
Reported 2006-07-14 06:40:31 PDT
Steps to reproduce: 1. Open Safari and set the User Agent to MSIE 6 from the Debug menu. 2. Access URL:  http://www.fckeditor.net/demo/default.html Expected results: Safari should not crash. Actual results: Safari crashes.
Attachments
Stack trace from crash (23.97 KB, text/plain)
2006-07-14 06:42 PDT, David Kilzer (:ddkilzer) 		no flags
Details
patch with detailed change log and a layout test (8.38 KB, patch)
2006-07-14 19:41 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2006-07-14 06:42:54 PDT
Created attachment 9447 [details] Stack trace from crash This crash was created using Safari 2.0.4 (419.3) with locally-built WebKit r15425 on Mac OS X 10.4.7 (8J135/PowerPC).
David Kilzer (:ddkilzer)
Comment 2 2006-07-14 06:47:02 PDT
Tested production Safari 2.0.4 (419.3) with WebKit 418.8 on Mac OS X 10.4.7 (8J135/PowerPC), and it does NOT crash when loading this URL and spoofing as MSIE 6.  Therefore, this is a regression.  Also added NeedsRadar keyword.  Crashers are also severity critical (I think).
David Kilzer (:ddkilzer)
Comment 3 2006-07-14 06:54:06 PDT
A non-MSIE-6-spoofing version was put up here as well (crashes the same way): http://blog.podemus.com/broadcast/fcktest/_samples/default.html
Joost de Valk (AlthA)
Comment 4 2006-07-14 07:05:28 PDT
I think this url: http://www.fckeditor.net/_temp/test_safari.html is better, as it doesn't require the spoof to get this crash. Changing subject and URL.
webkit
Comment 5 2006-07-14 08:06:04 PDT
The master Bug 9915 has been created for FCKeditor. This bug should depend on that one and should not block others. (or even mark this as duplicate)
Alice Liu
Comment 6 2006-07-14 17:17:05 PDT
<rdar://problem/4631837>
Darin Adler
Comment 7 2006-07-14 18:52:52 PDT
The crash is caused by setting the "dir" property on the document when it has no body. Crash can be fixed by adding a nil check.
Darin Adler
Comment 8 2006-07-14 19:41:14 PDT
Created attachment 9458 [details] patch with detailed change log and a layout test
John Sullivan
Comment 9 2006-07-15 09:23:50 PDT
Comment on attachment 9458 [details] patch with detailed change log and a layout test Sorry I didn't look in Bugzilla when I spurred Geoff into fixing this by adding comments to Radar. I think his fix is already in, but maybe we should take the layout tests? Up to you.
Geoffrey Garen
Comment 10 2006-07-15 09:26:51 PDT
Committed revision 15456.
Geoffrey Garen
Comment 11 2006-07-15 09:27:37 PDT
I passed on the layout test here because I didn't see a reason not to dumpAsText().
Darin Adler
Comment 12 2006-07-15 12:18:16 PDT
(In reply to comment #11) > I passed on the layout test here because I didn't see a reason not to > dumpAsText(). I think we should have a layout test that checks the behavior when you set dir before you have a body element and then get a body element. It would be good to match other browsers -- I assume they all would ignore any dir set without a body element. But that's beyond the scope of fixing this bug.
David Kilzer (:ddkilzer)
Comment 13 2006-07-15 21:33:39 PDT
(In reply to comment #12) > I think we should have a layout test that checks the behavior when you set dir > before you have a body element and then get a body element. It would be good to > match other browsers -- I assume they all would ignore any dir set without a > body element. But that's beyond the scope of fixing this bug. Filed Bug 9947 for this issue.
Note You need to log in before you can comment on or make changes to this bug.