38992 2010-05-12 08:00:44 -0700 REGRESSION: Crash by pasting to a textarea with white-space:nowrap 2010-05-17 18:22:40 -0700 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) All OS X 10.5 RESOLVED FIXED InRadar P1 Major --- 1 tkent tony abarth eric tony webkit.review.bot oldest_to_newest 224610 0 55847 tkent 2010-05-12 08:00:44 -0700 Created attachment 55847 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 224630 1 tkent 2010-05-12 08:29:59 -0700 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. 225175 2 55952 tony 2010-05-13 00:14:31 -0700 Created attachment 55952 Patch 225181 3 tony 2010-05-13 00:21:36 -0700 (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. 225460 4 55952 darin 2010-05-13 13:11:58 -0700 Comment on attachment 55952 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. 226155 5 abarth 2010-05-14 23:30:31 -0700 Attachment 55952 was posted by a committer and has review+, assigning to Tony Chang for commit. 226589 6 tony 2010-05-16 19:07:51 -0700 (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. 226605 7 tony 2010-05-16 21:09:12 -0700 Committed r59591: <http://trac.webkit.org/changeset/59591> 226619 8 webkit.review.bot 2010-05-16 22:16:36 -0700 http://trac.webkit.org/changeset/59591 might have broken GTK Linux 32-bit Debug 227032 9 adele 2010-05-17 18:22:40 -0700 <rdar://problem/7995593> 55847 2010-05-12 08:00:44 -0700 2010-05-12 08:00:44 -0700 Test case skem.html text/html 307 tkent PGh0bWw+CjxoZWFkPgo8L2hlYWQ+Cjxib2R5Pgo8cD5TZWxlY3QgYWxsIG9mIHRoZSBmaXJzdCB0 ZXh0YXJlYSBjb250ZW50LCB0aGVuIHBhc3RlIGl0IGludG8gdGhlIHNlY29uZCB0ZXh0YXJlYS48 L3A+Cjx0ZXh0YXJlYSBjb2xzPSI1NSIgcm93cz0iNCIgc3R5bGU9IndoaXRlLXNwZWFjZTpub3dy YXA7Ij4KYWFhCmJiYgo8L3RleHRhcmVhPjxici8+Cjx0ZXh0YXJlYSBuYW1lPSJjdXN0b20iIGNv bHM9IjU1IiByb3dzPSI0IiBzdHlsZT0id2hpdGUtc3BhY2U6bm93cmFwOyI+CjwvdGV4dGFyZWE+ PGJyLz4KPC9ib2R5Pgo8L2h0bWw+Cg== 55952 2010-05-13 00:14:31 -0700 2010-05-13 13:11:58 -0700 Patch bug-38992-20100513161429.patch text/plain 4491 tony ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCAwY2FhZWIwYmZiYzg2MDM2OGEzOThjNmVmMDg4YTU4MGU5ODY0MjViLi5jOGRjNGFh NTA0NTc2OTFiMGYzZDA3ZTViMmEwYTg3ODc0MzE4MWYwIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAK KzIwMTAtMDUtMTMgIFRvbnkgQ2hhbmcgIDx0b255QGNocm9taXVtLm9yZz4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBSRUdSRVNTSU9OOiBDcmFzaCBi eSBwYXN0aW5nIHRvIGEgdGV4dGFyZWEgd2l0aCB3aGl0ZS1zcGFjZTpub3dyYXAKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM4OTkyCisKKyAgICAgICAg KiBlZGl0aW5nL3Bhc3RlYm9hcmQvcGFzdGUtcGxhaW50ZXh0LW5vd3JhcC1leHBlY3RlZC50eHQ6 IEFkZGVkLgorICAgICAgICAqIGVkaXRpbmcvcGFzdGVib2FyZC9wYXN0ZS1wbGFpbnRleHQtbm93 cmFwLmh0bWw6IEFkZGVkLgorCiAyMDEwLTA1LTEyICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFz ZXJAYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhdmlkIEh5YXR0LgpkaWZmIC0t Z2l0IGEvTGF5b3V0VGVzdHMvZWRpdGluZy9wYXN0ZWJvYXJkL3Bhc3RlLXBsYWludGV4dC1ub3dy YXAtZXhwZWN0ZWQudHh0IGIvTGF5b3V0VGVzdHMvZWRpdGluZy9wYXN0ZWJvYXJkL3Bhc3RlLXBs YWludGV4dC1ub3dyYXAtZXhwZWN0ZWQudHh0Cm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLjk0ZmI0NzJiOGM3YzNmNmY5 NzQ5Y2Q2Y2ExYzY1MGRjMmQ5MWVmMzcKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9l ZGl0aW5nL3Bhc3RlYm9hcmQvcGFzdGUtcGxhaW50ZXh0LW5vd3JhcC1leHBlY3RlZC50eHQKQEAg LTAsMCArMSw2IEBACitDb3B5aW5nIGFuZCBwYXN0aW5nIHRoZSB0ZXh0IGJlbG93IGluIHBsYWNl IHNob3VsZCBub3QgY3Jhc2guCisKKworUEFTU0VECisKKwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVz dHMvZWRpdGluZy9wYXN0ZWJvYXJkL3Bhc3RlLXBsYWludGV4dC1ub3dyYXAuaHRtbCBiL0xheW91 dFRlc3RzL2VkaXRpbmcvcGFzdGVib2FyZC9wYXN0ZS1wbGFpbnRleHQtbm93cmFwLmh0bWwKbmV3 IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMC4uZWMzNmRhMzM3YWUzZmIxYTE3ZmQ0Njg0YzRlZjgyYWIxMzIwM2EwYgotLS0gL2Rl di9udWxsCisrKyBiL0xheW91dFRlc3RzL2VkaXRpbmcvcGFzdGVib2FyZC9wYXN0ZS1wbGFpbnRl eHQtbm93cmFwLmh0bWwKQEAgLTAsMCArMSwyMCBAQAorPGJvZHk+Cis8cD5Db3B5aW5nIGFuZCBw YXN0aW5nIHRoZSB0ZXh0IGJlbG93IGluIHBsYWNlIHNob3VsZCBub3QgY3Jhc2guPC9wPgorPHRl eHRhcmVhIGlkPSJ0ZXN0IiBzdHlsZT0id2hpdGUtc3BhY2U6bm93cmFwIj5saW5lMQorbGluZTIK KzwvdGV4dGFyZWE+Cis8cCBpZD0icmVzdWx0cyI+RkFJTEVEPC9wPgorPC9ib2R5PgorPHNjcmlw dCBzcmM9Ii4uL2VkaXRpbmcuanMiPjwvc2NyaXB0PgorPHNjcmlwdD4KK2Z1bmN0aW9uIGVkaXRp bmdUZXN0KCkKK3sKKyAgICBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgidGVzdCIpLmZvY3VzKCk7 CisgICAgZXhlY1NlbGVjdEFsbENvbW1hbmQoKTsKKyAgICBleGVjQ29weUNvbW1hbmQoKTsKKyAg ICBleGVjUGFzdGVDb21tYW5kKCk7CisgICAgZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoInJlc3Vs dHMiKS5pbm5lclRleHQgPSAiUEFTU0VEIjsKK30KKworcnVuRHVtcEFzVGV4dEVkaXRpbmdUZXN0 KGZhbHNlKTsKKzwvc2NyaXB0PgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9DaGFuZ2VMb2cgYi9XZWJD b3JlL0NoYW5nZUxvZwppbmRleCBlODI1YWUyODY1YTQwNDI1YWE1ZWQxMWIwNjA2ZDc2M2MxYzk4 ZGNlLi45NjA1OTMzMmM1NTkwNWE2MWI1MTczNjRlY2I4ODNiNTI5OTU0MGRhIDEwMDY0NAotLS0g YS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDEwLTA1LTEzICBUb255IENoYW5nICA8dG9ueUBjaHJvbWl1bS5vcmc+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUkVHUkVTU0lPTjogQ3Jh c2ggYnkgcGFzdGluZyB0byBhIHRleHRhcmVhIHdpdGggd2hpdGUtc3BhY2U6bm93cmFwCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zODk5MgorCisgICAg ICAgIFRlc3Q6IGVkaXRpbmcvcGFzdGVib2FyZC9wYXN0ZS1wbGFpbnRleHQtbm93cmFwLmh0bWwK KworICAgICAgICAqIGVkaXRpbmcvSW5zZXJ0UGFyYWdyYXBoU2VwYXJhdG9yQ29tbWFuZC5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpoaWdoZXN0VmlzdWFsbHlFcXVpdmFsZW50RGl2QmVsb3dSb290 KToKKyAgICAgICAgKFdlYkNvcmU6Okluc2VydFBhcmFncmFwaFNlcGFyYXRvckNvbW1hbmQ6OmRv QXBwbHkpOgorCiAyMDEwLTA1LTEyICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUu Y29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhdmlkIEh5YXR0LgpkaWZmIC0tZ2l0IGEvV2Vi Q29yZS9lZGl0aW5nL0luc2VydFBhcmFncmFwaFNlcGFyYXRvckNvbW1hbmQuY3BwIGIvV2ViQ29y ZS9lZGl0aW5nL0luc2VydFBhcmFncmFwaFNlcGFyYXRvckNvbW1hbmQuY3BwCmluZGV4IDA1OGI5 NjFhODBkOTkxNmRjMzI1NDg2NDU0YTk3OWY5YWY2Mjg2ODQuLmFmZmI2MzkwNDUxN2NkYzRhNGYy MWNlMjQ1NmZlMmMzMGQxZGU2ODQgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvZWRpdGluZy9JbnNlcnRQ YXJhZ3JhcGhTZXBhcmF0b3JDb21tYW5kLmNwcAorKysgYi9XZWJDb3JlL2VkaXRpbmcvSW5zZXJ0 UGFyYWdyYXBoU2VwYXJhdG9yQ29tbWFuZC5jcHAKQEAgLTQ4LDEwICs0OCwxMiBAQCB1c2luZyBu YW1lc3BhY2UgSFRNTE5hbWVzOwogLy8gcGFzdGluZywgaXQncyBlYXN5IHRvIGhhdmUgZWFjaCBu ZXcgbGluZSBiZSBhIGRpdiBkZWVwZXIgdGhhbiB0aGUgcHJldmlvdXMuICBFLmcuLCBpbiB0aGUg Y2FzZQogLy8gYmVsb3csIHdlIHdhbnQgdG8gaW5zZXJ0IGF0IF4gaW5zdGVhZCBvZiB8LgogLy8g PGRpdj5mb288ZGl2PmJhcjwvZGl2Pnw8L2Rpdj5eCi1zdGF0aWMgRWxlbWVudCogaGlnaGVzdFZp c3VhbGx5RXF1aXZhbGVudERpdihFbGVtZW50KiBzdGFydEJsb2NrKQorc3RhdGljIEVsZW1lbnQq IGhpZ2hlc3RWaXN1YWxseUVxdWl2YWxlbnREaXZCZWxvd1Jvb3QoRWxlbWVudCogc3RhcnRCbG9j aykKIHsKICAgICBFbGVtZW50KiBjdXJCbG9jayA9IHN0YXJ0QmxvY2s7Ci0gICAgd2hpbGUgKCFj dXJCbG9jay0+bmV4dFNpYmxpbmcoKSAmJiBjdXJCbG9jay0+cGFyZW50RWxlbWVudCgpLT5oYXNU YWdOYW1lKGRpdlRhZykpIHsKKyAgICAvLyBXZSBkb24ndCB3YW50IHRvIHJldHVybiBhIHJvb3Qg bm9kZSAoaWYgaXQgaGFwcGVucyB0byBiZSBhIGRpdiwgZS5nLiwgaW4gYSBkb2N1bWVudCBmcmFn bWVudCkgYmVjYXVzZSB0aGVyZSBhcmUgbm8KKyAgICAvLyBzaWJsaW5ncyBmb3IgdXMgdG8gYXBw ZW5kIHRvLgorICAgIHdoaWxlICghY3VyQmxvY2stPm5leHRTaWJsaW5nKCkgJiYgY3VyQmxvY2st PnBhcmVudEVsZW1lbnQoKS0+aGFzVGFnTmFtZShkaXZUYWcpICYmIGN1ckJsb2NrLT5wYXJlbnRF bGVtZW50KCktPnBhcmVudEVsZW1lbnQoKSkgewogICAgICAgICBOYW1lZE5vZGVNYXAqIGF0dHJp YnV0ZXMgPSBjdXJCbG9jay0+cGFyZW50RWxlbWVudCgpLT5hdHRyaWJ1dGVzKHRydWUpOwogICAg ICAgICBpZiAoYXR0cmlidXRlcyAmJiAhYXR0cmlidXRlcy0+aXNFbXB0eSgpKQogICAgICAgICAg ICAgYnJlYWs7CkBAIC0yMzUsNyArMjM3LDcgQEAgdm9pZCBJbnNlcnRQYXJhZ3JhcGhTZXBhcmF0 b3JDb21tYW5kOjpkb0FwcGx5KCkKICAgICAgICAgICAgIC8vIGZvciBkaXYgbm9kZXMsIHRoaXMg Y2FuIHJlc3VsdCBpbiBuZXN0ZWQgZGl2IHRhZ3MgdGhhdCBhcmUgaGFyZCB0byBicmVhayBvdXQg b2YuCiAgICAgICAgICAgICBFbGVtZW50KiBzaWJsaW5nTm9kZSA9IHN0YXJ0QmxvY2s7CiAgICAg ICAgICAgICBpZiAoYmxvY2tUb0luc2VydC0+aGFzVGFnTmFtZShkaXZUYWcpKQotICAgICAgICAg ICAgICAgIHNpYmxpbmdOb2RlID0gaGlnaGVzdFZpc3VhbGx5RXF1aXZhbGVudERpdihzdGFydEJs b2NrKTsKKyAgICAgICAgICAgICAgICBzaWJsaW5nTm9kZSA9IGhpZ2hlc3RWaXN1YWxseUVxdWl2 YWxlbnREaXZCZWxvd1Jvb3Qoc3RhcnRCbG9jayk7CiAgICAgICAgICAgICBpbnNlcnROb2RlQWZ0 ZXIoYmxvY2tUb0luc2VydCwgc2libGluZ05vZGUpOwogICAgICAgICB9CiAK