Bug 10127 – REGRESSION: Crash undoing delete in textarea

• Component: HTML Editing
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

After typing on the order of 4000 characters into an initially-blank textarea (I was replying to a message), I tried to undo a highlight-and-delete operation in the text area, only to have WebKit crash (losing all of my text).

Unfortunately, I don't know how to reproduce this yet.  I do have a crash log that I will post next, though.

This occurred on a locally-built debug build of WebKit r15636 on Mac OS X 10.4.7 (8J135/PowerPC).

[David Kilzer (:ddkilzer)]

Created attachment 9711 [details]
Crash log (r15636)

[Darin Adler]

I'm not sure a non-reproducible bug should be P1. But maybe it's just a matter of time until we reproduce this.

[David Kilzer (:ddkilzer)]

Changed to P2 per Comment #2 until it is reproducible.

[Stephanie Lewis]

4826990

[David Kilzer (:ddkilzer)]

Created attachment 12699 [details]
Crash log (r19165)

Tried to reproduce this with a locally-built debug build of WebKit r19165 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037) and was able to do it!  This attachment is the stack trace.

I basically typed random garbage in a Bugzilla comments textarea, then would tab-out and tab-in to the textarea every now and then to force another item to be added to the undo manager.  Eventually I tabbed back in and tried to immediately undo some typing and it crashed.  I'll see if I can come up with a way to reproduce this reliably.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> Changed to P2 per Comment #2 until it is reproducible.

Back to P1 per Comment #5.

[David Kilzer (:ddkilzer)]

Steps to reproduce!

1. Open Safari/WebKit.
2. Open the URL:  data:text/html,<textarea>
3. Click in the textarea to give it focus and get a caret.
4. Begin typing gibberish, but DO NOT hit Return.  DO put spaces in the gibberish so that the lines may word-wrap.
5. Type until you're in the middle of the third line (the top line will have scrolled off, and a scrollbar placeholder will have appeared on the right of the textarea).
6. Tab out of the textarea so it loses focus.  (Use Tab or Shift-Tab.)
7. Tab back into the textarea so it regains focus.  (Use the opposite tab operation from Step #6.)
8. Hit Cmd-Z to undo the last operation.

Expected results:

The three lines of typing should be undone.

Actual results:

Safari/WebKit crashes.

Notes:

This is also reproducible with a larger textarea like those used for comments in Bugzilla, but you only need to type two lines of gibberish instead of three (apparently because the scrollbar area is already defined?).

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> This is also reproducible with a larger textarea like those used for comments
> in Bugzilla, but you only need to type two lines of gibberish instead of three
> (apparently because the scrollbar area is already defined?).

It has nothing to do with the scollbar area since two lines in a Bugzilla comment don't produce that.  Perhaps it's the amount of text that's been typed.

Note that if you hit Return before unfocusing/focusing, the undo operation works fine!

[Sam Weinig]

David, are you still able to reproduce this?  I tried following your steps numerous times and was unable to get WebKit to crash.

[David Kilzer (:ddkilzer)]

Yes, I am still able to reproduce using a local debug build of WebKit r19621 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127).  I will rebuild and try with r19636 next.  Stack trace from r19621:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000014

Thread 0 Crashed:
0   com.apple.WebCore        	0x0162bd80 WebCore::Shared<WebCore::StringImpl>::ref() + 36 (Shared.h:41)
1   com.apple.WebCore        	0x0162fbd8 WTF::RefPtr<WebCore::StringImpl>::RefPtr[in-charge](WTF::RefPtr<WebCore::StringImpl> const&) + 80 (RefPtr.h:37)
2   com.apple.WebCore        	0x01630fe8 WebCore::String::String[in-charge](WebCore::String const&) + 48 (PlatformString.h:51)
3   com.apple.WebCore        	0x0164753c WebCore::DocumentMarker::DocumentMarker[in-charge](WebCore::DocumentMarker const&) + 104 (DocumentMarker.h:33)
4   com.apple.WebCore        	0x011412cc WebCore::Document::removeMarkers(WebCore::Node*, unsigned, int, WebCore::DocumentMarker::MarkerType) + 320 (Document.cpp:2865)
5   com.apple.WebCore        	0x012b4320 WebCore::CharacterData::deleteData(unsigned, unsigned, int&) + 388 (CharacterData.cpp:178)
6   com.apple.WebCore        	0x01280b20 WebCore::InsertIntoTextNodeCommand::doUnapply() + 352 (InsertIntoTextNodeCommand.cpp:62)
7   com.apple.WebCore        	0x0127a0c8 WebCore::EditCommand::unapply() + 276 (EditCommand.cpp:113)
8   com.apple.WebCore        	0x0126fd94 WebCore::CompositeEditCommand::doUnapply() + 112 (CompositeEditCommand.cpp:78)
9   com.apple.WebCore        	0x0127a0c8 WebCore::EditCommand::unapply() + 276 (EditCommand.cpp:113)
10  com.apple.WebCore        	0x0126fd94 WebCore::CompositeEditCommand::doUnapply() + 112 (CompositeEditCommand.cpp:78)
11  com.apple.WebCore        	0x0127a0c8 WebCore::EditCommand::unapply() + 276 (EditCommand.cpp:113)
12  com.apple.WebKit         	0x003c5968 -[WebEditorUndoTarget undoEditing:] + 224 (WebEditorClient.mm:124)
13  com.apple.Foundation     	0x929fd190 -[_NSUndoStack popAndInvoke] + 268
14  com.apple.Foundation     	0x929fd034 -[NSUndoManager undoNestedGroup] + 328
15  com.apple.AppKit         	0x937acc4c -[NSApplication sendAction:to:from:] + 108
16  com.apple.Safari         	0x0002956c 0x1000 + 165228
17  com.apple.AppKit         	0x938074b8 -[NSMenu performActionForItemAtIndex:] + 392
18  com.apple.AppKit         	0x9380723c -[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:] + 104
19  com.apple.AppKit         	0x93806ce4 -[NSMenu performKeyEquivalent:] + 272
20  com.apple.AppKit         	0x93806930 -[NSApplication _handleKeyEquivalent:] + 328
21  com.apple.AppKit         	0x93710408 -[NSApplication sendEvent:] + 2944
22  com.apple.Safari         	0x00021238 0x1000 + 131640
23  com.apple.AppKit         	0x93707d10 -[NSApplication run] + 508
24  com.apple.AppKit         	0x937f887c NSApplicationMain + 452
25  com.apple.Safari         	0x0005c77c 0x1000 + 374652
26  com.apple.Safari         	0x0005c624 0x1000 + 374308

[David Kilzer (:ddkilzer)]

A local debug build of WebKit r19636 crashes as well.

I can't reproduce. Dave, are you able to reproduce the bug regardless of whether "check spelling as you type" is enabled, or is it reproducible only with one of the settings?

[David Kilzer (:ddkilzer)]

(In reply to comment #12)
> I can't reproduce. Dave, are you able to reproduce the bug regardless of
> whether "check spelling as you type" is enabled, or is it reproducible only
> with one of the settings?

I do have "check spelling as you type" enabled when doing this, and I'm typing misspelled words like "asdf asdf a sf asf ae fa sdf asdf asdf sa".

The current build I'm using (r19636) doesn't seem to let me change that setting anymore, either.  I'm going to have to find an older nightly build to disable the setting to test this.

[David Kilzer (:ddkilzer)]

(In reply to comment #12)
> I can't reproduce. Dave, are you able to reproduce the bug regardless of
> whether "check spelling as you type" is enabled, or is it reproducible only
> with one of the settings?

I can confirm that there is no crash when "check spelling as you type" is disabled.  As mentioned previously, it will crash if "check spelling as you type" is enabled.

(In reply to comment #13)
> The current build I'm using (r19636) doesn't seem to let me change that setting
> anymore, either.

a) that's sort of being tracked by bug 12761
b) you can use Edit > Spelling > Check Spelling As You Type from Safari's menu bar

> I do have "check spelling as you type" enabled when doing this, and I'm typing
> misspelled words like "asdf asdf a sf asf ae fa sdf asdf asdf sa".

Oops, when trying to reproduce, I misread "DO put spaces" as "DO NOT put spaces". However, even now that I put spaces, I cannot reproduce the crash.

What spelling dictionary are you using? Apple's English or something else?

I finally reproduced the bug. Setting a breakpoint in Document::removeMarkers() and stepping into the initialization of the local variable marker, I ended up in the constructor RefPtr<StringImpl>::RefPtr(const RefPtr& o), which I don't know what to make of. I don't know where o's contents come from.

[David Kilzer (:ddkilzer)]

(In reply to comment #15)
> What spelling dictionary are you using? Apple's English or something else?

I must be using Apple's English dictionary because I've never changed it to anything else.

Created attachment 13185 [details]
[WIP] Don't abuse Vector iterators

The problem seems to be that a Vector iterator (essentially a pointer) was begin used on a Vector while inserting objects into it, which caused its buffer to move to a different address, rendering the iterator invalid. The solution is to use an index instead of a fancy iterator. No change log or regression test yet.

[Darin Adler]

Comment on attachment 13185 [details]
[WIP] Don't abuse Vector iterators

r=me

[Darin Adler]

Comment on attachment 13185 [details]
[WIP] Don't abuse Vector iterators

Looks great. I set review+ but I wasn't supposed to review this.

Created attachment 13187 [details]
Replace all instances of Vector<DocumentMarker>::iterator with indices

No layout test regressions. I haven't figured out how to make a regression test to go with this patch.

[Darin Adler]

Comment on attachment 13187 [details]
Replace all instances of Vector<DocumentMarker>::iterator with indices

r=me

[Sam Weinig]

Landed in r19643!