Bug 38992 – REGRESSION: Crash by pasting to a textarea with white-space:nowrap

• Component: HTML Editing
• Status: RESOLVED FIXED

[Kent Tamura]

Created attachment 55847 [details]
Test case

A crash issue in http://code.google.com/p/chromium/issues/detail?id=43977

See the attached test case.
r54395 is suspicious.


Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000060
0x00000001010be999 in WebCore::QualifiedName::matches (this=0x60, other=@0x102bdc948) at QualifiedName.h:74
74          bool matches(const QualifiedName& other) const { return m_impl == other.m_impl || (localName() == other.localName() && namespaceURI() == other.namespaceURI()); }
(gdb) bt
#0  0x00000001010be999 in WebCore::QualifiedName::matches (this=0x60, other=@0x102bdc948) at QualifiedName.h:74
#1  0x00000001010bea29 in WebCore::Element::hasTagName (this=0x0, tagName=@0x102bdc948) at Element.h:161
#2  0x00000001013ca81e in WebCore::highestVisuallyEquivalentDiv (startBlock=0x121425330) at /Users/kent/WebKit/WebCore/editing/InsertParagraphSeparatorCommand.cpp:54
#3  0x00000001013cb2d1 in WebCore::InsertParagraphSeparatorCommand::doApply (this=0x121426fc0) at /Users/kent/WebKit/WebCore/editing/InsertParagraphSeparatorCommand.cpp:238
#4  0x000000010120ed71 in WebCore::EditCommand::apply (this=0x121426fc0) at /Users/kent/WebKit/WebCore/editing/EditCommand.cpp:91
#5  0x0000000100ffa67e in WebCore::CompositeEditCommand::applyCommandToComposite (this=0x12157fd70, cmd=@0x7fff5fbfe110) at /Users/kent/WebKit/WebCore/editing/CompositeEditCommand.cpp:99
#6  0x0000000100ffe064 in WebCore::CompositeEditCommand::insertParagraphSeparator (this=0x12157fd70, useDefaultParagraphElement=true) at /Users/kent/WebKit/WebCore/editing/CompositeEditCommand.cpp:125
#7  0x00000001018909f9 in WebCore::ReplaceSelectionCommand::doApply (this=0x12157fd70) at /Users/kent/WebKit/WebCore/editing/ReplaceSelectionCommand.cpp:1036
#8  0x000000010120ed71 in WebCore::EditCommand::apply (this=0x12157fd70) at /Users/kent/WebKit/WebCore/editing/EditCommand.cpp:91
#9  0x000000010120edec in WebCore::applyCommand (command=@0x7fff5fbfec70) at /Users/kent/WebKit/WebCore/editing/EditCommand.cpp:212
#10 0x0000000101217e79 in WebCore::Editor::replaceSelectionWithFragment (this=0x1190016f0, fragment=@0x7fff5fbfecf0, selectReplacement=false, smartReplace=false, matchStyle=true) at /Users/kent/WebKit/WebCore/editing/Editor.cpp:311
#11 0x0000000101217f08 in WebCore::Editor::replaceSelectionWithText (this=0x1190016f0, text=@0x7fff5fbfed60, selectReplacement=false, smartReplace=false) at /Users/kent/WebKit/WebCore/editing/Editor.cpp:317
#12 0x00000001012187f2 in WebCore::Editor::pasteAsPlainTextWithPasteboard (this=0x1190016f0, pasteboard=0x119851da0) at /Users/kent/WebKit/WebCore/editing/Editor.cpp:277
#13 0x000000010121883e in WebCore::Editor::pasteAsPlainText (this=0x1190016f0) at /Users/kent/WebKit/WebCore/editing/Editor.cpp:1080
#14 0x0000000100382357 in -[WebHTMLView(WebInternal) paste:] (self=0x11bca5cb0, _cmd=0x7fff86a148e8, sender=0x106a1a4b0) at /Users/kent/WebKit/WebKit/mac/WebView/WebHTM

[Kent Tamura]

Correction of the instruction in the attachment:
  Select all of the first textarea content, then paste it into the second textarea.
->
  Select all of the first textarea content, copy it, then paste it into the second textarea.

[Tony Chang]

Created attachment 55952 [details]
Patch

[Tony Chang]

(In reply to comment #2)
> Created an attachment (id=55952) [details]
> Patch

The crash happens if we're pasting plain text into a white-space:nowrap field.  It happens in code added in r54395, as Kent mentions.  The code added didn't consider the possibility that we could be operating on a document fragment where the root element is a <div>.  When pasting, we create a document fragment from the clipboard text that triggers the crash.

[Darin Adler]

Comment on attachment 55952 [details]
Patch

Not new to this patch: It's very strange that this code specifically special-cases <div> elements. There's nothing unqiue about a <div> elements, is there? Can't a <p> elements perform almost the identical function?

The check that an element has a grandparent seems to be a roundabout way to define a "root node" that depends on the one hand on the fact that we use document fragments with single nodes in them and on the other hand on the fact that a document has an HTML element in it. I'd think that in the case of a document we'd not want to add a sibling to a body node either, so I think the parent->parent check is not ideal.

But r=me on this change for now, since it does some good and no harm.

I'd like further thought and testing, though, since this seems a little loose.

[Adam Barth]

Attachment 55952 [details] was posted by a committer and has review+, assigning to Tony Chang for commit.

[Tony Chang]

(In reply to comment #4)
> (From update of attachment 55952 [details])
> Not new to this patch: It's very strange that this code specifically special-cases <div> elements. There's nothing unqiue about a <div> elements, is there? Can't a <p> elements perform almost the identical function?

I've created bug 39193 for this.

> The check that an element has a grandparent seems to be a roundabout way to define a "root node" that depends on the one hand on the fact that we use document fragments with single nodes in them and on the other hand on the fact that a document has an HTML element in it. I'd think that in the case of a document we'd not want to add a sibling to a body node either, so I think the parent->parent check is not ideal.

I agree that we would not want to add a sibling to a body node, but this code specifically checks for div tags so it'll only create siblings for divs.

> But r=me on this change for now, since it does some good and no harm.
> 
> I'd like further thought and testing, though, since this seems a little loose.

I'll see if I can come up with some more tests for this-- it may be the case that we just shouldn't run this code for document fragments at all, although it seems harmless right now.

[Tony Chang]

Committed r59591: <http://trac.webkit.org/changeset/59591>

[WebKit Review Bot]

http://trac.webkit.org/changeset/59591 might have broken GTK Linux 32-bit Debug

[Adele Peterson]

<rdar://problem/7995593>