Bug 9911 - REGRESSION: Reproducible crash on FCKeditor demo
Summary: REGRESSION: Reproducible crash on FCKeditor demo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P1 Critical
Assignee: Darin Adler
URL: http://www.fckeditor.net/_temp/test_s...
Keywords: InRadar, NeedsReduction, Regression
Depends on:
Blocks: 9915
  Show dependency treegraph
 
Reported: 2006-07-14 06:40 PDT by David Kilzer (:ddkilzer)
Modified: 2006-07-15 21:33 PDT (History)
0 users

See Also:


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 Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 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.
Comment 1 David Kilzer (:ddkilzer) 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).
Comment 2 David Kilzer (:ddkilzer) 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).

Comment 3 David Kilzer (:ddkilzer) 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
Comment 4 Joost de Valk (AlthA) 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.
Comment 5 Frederico Caldeira Knabben 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)
Comment 6 Alice Liu 2006-07-14 17:17:05 PDT
 <rdar://problem/4631837>
Comment 7 Darin Adler 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.
Comment 8 Darin Adler 2006-07-14 19:41:14 PDT
Created attachment 9458 [details]
patch with detailed change log and a layout test
Comment 9 John Sullivan 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.
Comment 10 Geoffrey Garen 2006-07-15 09:26:51 PDT
Committed revision 15456.
Comment 11 Geoffrey Garen 2006-07-15 09:27:37 PDT
I passed on the layout test here because I didn't see a reason not to dumpAsText().
Comment 12 Darin Adler 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.
Comment 13 David Kilzer (:ddkilzer) 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.