Bug 9911 – REGRESSION: Reproducible crash on FCKeditor demo

• Component: DOM
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

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.

[David Kilzer (:ddkilzer)]

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)]

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)]

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)]

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.

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]

 <rdar://problem/4631837>

[Darin Adler]

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]

Created attachment 9458 [details]
patch with detailed change log and a layout test

[John Sullivan]

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]

Committed revision 15456.

[Geoffrey Garen]

I passed on the layout test here because I didn't see a reason not to dumpAsText().

[Darin Adler]

(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)]

(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.