Bug 8616 - REGRESSION: TinyMCE: Crash on Undo
Summary: REGRESSION: TinyMCE: Crash on Undo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Justin Garcia
URL: http://tinymce.moxiecode.com/example....
Keywords: InRadar, Regression
: 8807 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-04-26 16:35 PDT by Justin Garcia
Modified: 2006-06-08 09:38 PDT (History)
4 users (show)

See Also:


Attachments
patch (37.79 KB, patch)
2006-06-04 15:45 PDT, Darin Adler
justin.garcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Garcia 2006-04-26 16:35:27 PDT
Goto http://tinymce.moxiecode.com/example.php?example=true
Select a bit of text in the editable area
Drag and drop it somewhere in the editable area
Undo

Thread 0 Crashed:
0   <<00000000>> 	0x02211380 0 + 35722112
1   com.apple.WebCore        	0x013b5280 WebCore::rendererAfterPosition(WebCore::RenderObject*, unsigned) + 32
2   com.apple.WebCore        	0x013b6620 WebCore::RenderCanvas::setSelection(WebCore::RenderObject*, int, WebCore::RenderObject*, int) + 288
3   com.apple.WebCore        	0x0131790c WebCore::Document::updateSelection() + 92
4   com.apple.WebCore        	0x012f3988 WebCore::Frame::setSelection(WebCore::SelectionController const&, bool, bool) + 600
5   com.apple.WebCore        	0x012f8df8 WebCore::Frame::unappliedEditing(WebCore::EditCommandPtr&) + 344
6   com.apple.WebCore        	0x0142a424 WebCore::EditCommand::unapply() + 116

The RenderCanvas's selection endpoints are stale pointers.  There is code that clears the canvas's endpoints if one of them is about to be destroyed, that must not be getting called.  The code that nulls out the SelectionController also isn't being called, apparently because the editable area is in a subframe, and the main frame's SelectionController is the one that should be notified of the removal, but only the subframe's SC is notified.
Comment 1 Justin Garcia 2006-04-26 16:36:44 PDT
We think we could have one SC per page, instead of one per frame.  That would help.
Comment 2 Darin Adler 2006-04-27 07:51:11 PDT
(In reply to comment #1)
> We think we could have one SC per page, instead of one per frame.  That would
> help.

That sounds good. But there may be some circumstances where we want to support a selection in more than one frame. For example, there can be an active selection in an active frame and an inactive selection in an inactive frame. In general, Safari uses WebView in the "one selection in entire window" mode, but Mail, for example, uses WebView in the "one active selection and one or more inactive selections in a window" mode.

Because of the Mail use case, I'm not sure we need to support multiple selections in a tree, but I'm not absolute sure we don't, assuming you were using a web page to implement the Mail application with a design mode frame as the document.
Comment 3 Justin Garcia 2006-05-09 10:30:27 PDT
*** Bug 8807 has been marked as a duplicate of this bug. ***
Comment 4 Justin Garcia 2006-05-09 19:30:45 PDT
*** Bug 7151 has been marked as a duplicate of this bug. ***
Comment 5 Alice Liu 2006-05-16 09:34:44 PDT
<rdar://problem/4549717>
Comment 6 Darin Adler 2006-06-04 12:45:28 PDT
I believe the primary issue here is that we end up with selections that point to nodes in other documents. Currently, any selection must refer only to nodes in that frame's document.

There are two ways to resolve this crash without major redesign. One is to fix things so that invariant is maintained. Another is to have the "node removed" code work on the parent frames too (or the entire frame tree).
Comment 7 Darin Adler 2006-06-04 15:45:22 PDT
Created attachment 8703 [details]
patch
Comment 8 Justin Garcia 2006-06-07 14:33:00 PDT
Comment on attachment 8703 [details]
patch

I think this might also fix 7165.  r=me
Comment 9 Darin Adler 2006-06-08 09:38:46 PDT
Committed revision 14770.