67668 2011-09-06 13:29:05 -0700 Crashes in WebCore::EditCommand::apply(), DeleteSelectionCommand::doApply() 2011-09-12 01:43:14 -0700 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) All All RESOLVED FIXED P2 Major --- 67762 67763 67765 67766 67767 1 inferno webkit-unassigned cachobot cdn rniwa shinyak oldest_to_newest 462801 0 inferno 2011-09-06 13:29:05 -0700 testcase1:: <feSpotLight><sub id="div" contenteditable="true"><script> var sel = window.getSelection(); sel.setPosition(div, 0); document.execCommand("InsertHTML", false, "<dl>"); </script> testcase2:: ><meter contenteditable><span id="wrapper">><script> var sel = window.getSelection(); sel.setPosition(document.getElementById("wrapper"), 1); document.execCommand("InsertParagraph", false, null); </script> testcase3:: <div contenteditable="true" id="div"><hkern><span contenteditable="false"><dl>000A0<script> var sel = window.getSelection(); sel.setPosition(div, 2000000000); document.execCommand("Delete"); </script> These might be contributing to the top crashers in chromium 462805 1 inferno 2011-09-06 13:31:11 -0700 testcase4:: [WebCore::ApplyStyleCommand::doApply()] <card id="edit" contentEditable="true">A<script> edit.focus(); document.execCommand("SelectAll"); document.execCommand("RemoveFormat"); </script> testcase5:: [WebCore::ApplyStyleCommand::applyBlockStyle(WebCore::EditingStyle*) ] <mfrac id="div" contenteditable="true">&bcong;<script> div.focus(); document.execCommand("SelectAll"); document.execCommand("RemoveFormat"); </script> testcase6:: [WebCore::AppendNodeCommand::create(WTF::PassRefPtr<WebCore::ContainerNode>, WTF::PassRefPtr<WebCore::Node>) ] <script> function runTest() { window.getSelection().setBaseAndExtent(start, 0, null, 0); document.execCommand("Indent"); } </script> <meta content="2"/><body onLoad="runTest();"> ><defs contenteditable="true" id="start"> <rt>AAAAAAA0A0AAAA00 463898 2 106698 shinyak 2011-09-07 23:19:38 -0700 Created attachment 106698 Patch for testcase1 463899 3 106698 tkent 2011-09-07 23:28:30 -0700 Comment on attachment 106698 Patch for testcase1 View in context: https://bugs.webkit.org/attachment.cgi?id=106698&action=review > LayoutTests/ChangeLog:11 > + * editing/inserting/insert-67668-crash-expected.txt: Added. > + * editing/inserting/insert-67668-crash.html: Added. I do not like such cryptic test name. 463900 4 inferno 2011-09-07 23:29:39 -0700 Please keep the fixing bandwagon on remaining testcases and also merging to m14 835 branch. 463902 5 106698 rniwa 2011-09-07 23:31:08 -0700 Comment on attachment 106698 Patch for testcase1 (In reply to comment #3) > (From update of attachment 106698 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=106698&action=review > > > LayoutTests/ChangeLog:11 > > + * editing/inserting/insert-67668-crash-expected.txt: Added. > > + * editing/inserting/insert-67668-crash.html: Added. > > I do not like such cryptic test name. I agree. Please fix the filename before landing it. 463906 6 shinyak 2011-09-07 23:37:48 -0700 (In reply to comment #5) > (From update of attachment 106698 [details]) > (In reply to comment #3) > > (From update of attachment 106698 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=106698&action=review > > > > > LayoutTests/ChangeLog:11 > > > + * editing/inserting/insert-67668-crash-expected.txt: Added. > > > + * editing/inserting/insert-67668-crash.html: Added. > > > > I do not like such cryptic test name. > > I agree. Please fix the filename before landing it. Since the reason of these crashes are different, I would like to divide this bug into several bugs first. Then I upload patches again... 463907 7 inferno 2011-09-07 23:39:43 -0700 (In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 106698 [details] [details]) > > (In reply to comment #3) > > > (From update of attachment 106698 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=106698&action=review > > > > > > > LayoutTests/ChangeLog:11 > > > > + * editing/inserting/insert-67668-crash-expected.txt: Added. > > > > + * editing/inserting/insert-67668-crash.html: Added. > > > > > > I do not like such cryptic test name. > > > > I agree. Please fix the filename before landing it. > > Since the reason of these crashes are different, I would like to divide this bug into several bugs first. > Then I upload patches again... I just wanted to make your merging task easier. :) 463911 8 rniwa 2011-09-07 23:41:02 -0700 (In reply to comment #6) > Since the reason of these crashes are different, I would like to divide this bug into several bugs first. > Then I upload patches again... Sounds good to me. 463912 9 shinyak 2011-09-07 23:43:48 -0700 (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > (From update of attachment 106698 [details] [details] [details]) > > > (In reply to comment #3) > > > > (From update of attachment 106698 [details] [details] [details] [details]) > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=106698&action=review > > > > > > > > > LayoutTests/ChangeLog:11 > > > > > + * editing/inserting/insert-67668-crash-expected.txt: Added. > > > > > + * editing/inserting/insert-67668-crash.html: Added. > > > > > > > > I do not like such cryptic test name. > > > > > > I agree. Please fix the filename before landing it. > > > > Since the reason of these crashes are different, I would like to divide this bug into several bugs first. > > Then I upload patches again... > > I just wanted to make your merging task easier. :) I think we can easily track bugs by seeing 'Depends on' field. 464343 10 cdn 2011-09-08 14:10:13 -0700 I am not convinced that the core issue here is actually separate bugs. It seems like there is a common theme with all of these. More specifically each of the test cases involves creating a new selection and then calling set position on it (some call setposition as a result of a different call.. testcase 6 etc) I suspect we will need to figure out the underlying issue for some of these as a simple null check still hits tons of asserts for a lot of them. This isn't an argument against the patch just noting that I think there is a better common fix. 464355 11 rniwa 2011-09-08 14:15:56 -0700 (In reply to comment #10) > I am not convinced that the core issue here is actually separate bugs. It seems like there is a common theme with all of these. More specifically each of the test cases involves creating a new selection and then calling set position on it (some call setposition as a result of a different call.. testcase 6 etc) I'm confused. var sel = window.getSelection(); doesn't create a new selection. It obtains the selection of the current frame. And sel.setPosition(div, 0) modifies the selection of the current frame. As far as I can tell, most of these test cases are completely unrelated. 464627 12 inferno 2011-09-08 21:11:38 -0700 just a fyi, we need to merge these to both m14 and m15 branches. 464929 13 inferno 2011-09-09 11:21:06 -0700 Guys, can you handle some more. It is awesome to knock these null ptrs 1) <dl><div id="div" contenteditable="true"A><script> div.focus(); document.execCommand("InsertUnorderedList"); </script> Crash WebCore::InsertListCommand::unlistifyParagraph(WebCore::VisiblePosition const&, WebCore::HTMLElement*, WebCore::Node*) WebCore::InsertListCommand::doApplyForSingleParagraph(bool, WebCore::QualifiedName const&, WebCore::Range*) WebCore::InsertListCommand::doApply() WebCore::EditCommand::apply() WebCore::executeInsertUnorderedList(WebCore::Frame*, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) third_party/WebKit/Source/WebCore/editing/EditorCommand.cpp:0 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 2) ><a id="anchor" href="http://www.google.com/"><feDisplacementMap>A0A0AAAA0AA<base id="paste" contenteditable="true"><br><script> var sel = window.getSelection(); var range = document.createRange(); range.selectNodeContents(anchor); sel.addRange(range); document.execCommand("Copy"); paste.focus(); document.execCommand("Paste"); </script> WebCore::nextCandidate(WebCore::Position const&) WebCore::ReplaceSelectionCommand::positionAtStartOfInsertedContent() WebCore::ReplaceSelectionCommand::doApply() WebCore::EditCommand::apply() WebCore::Editor::replaceSelectionWithFragment(WTF::PassRefPtr<WebCore::DocumentFragment>, bool, bool, bool) WebCore::Editor::handleTextEvent(WebCore::TextEvent*) 464999 14 cdn 2011-09-09 13:20:10 -0700 Ryosuke: DOMSelection* DOMWindow::getSelection() { if (!m_selection) m_selection = DOMSelection::create(m_frame); ... When there is no m_selection on the DOMWindow a new DOMSelection is created. This is the case with all of these that I have stepped through. 465001 15 rniwa 2011-09-09 13:29:32 -0700 (In reply to comment #14) > DOMSelection* DOMWindow::getSelection() > { > if (!m_selection) > m_selection = DOMSelection::create(m_frame); > ... > > When there is no m_selection on the DOMWindow a new DOMSelection is created. This is the case with all of these that I have stepped through. Yes, but DOMSelection is just a wrapper for FrameSelection, which is a different object from DOMSelection and is always available on frame. 465006 16 cdn 2011-09-09 13:43:08 -0700 Ah, no worries. 465473 17 shinyak 2011-09-11 21:00:08 -0700 (In reply to comment #13) > Guys, can you handle some more. It is awesome to knock these null ptrs I could repro (1), but I couldn't repro (2). So I've filed (1) bug only. (Bug 67918) 465483 18 rniwa 2011-09-11 22:07:40 -0700 Let's not make this bug a master bug for all editing crashes. Please file new bugs for new crashes. 465531 19 shinyak 2011-09-12 01:43:14 -0700 (In reply to comment #18) > Let's not make this bug a master bug for all editing crashes. Please file new bugs for new crashes. OK. The all bugs have been closed now, so I think we should close this bug. 106698 2011-09-07 23:19:38 -0700 2011-09-08 00:10:16 -0700 Patch for testcase1 bug-67668-20110908151937.patch text/plain 3590 shinyak U3VidmVyc2lvbiBSZXZpc2lvbjogOTQ3NDEKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwppbmRleCAzYWEyMjNhZTRmMzYwYjIyZTM4YTdj MmY3YmExMzZjMmMyNDM2MTM1Li5jY2E0OTRiZTRiMjVlNDEwNDhiNTlkNmI3NjQ2MzRlZjljYWFl MjdmIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDktMDcgIFNoaW55YSBLYXdhbmFrYSAg PHNoaW55YWtAZ29vZ2xlLmNvbT4KKworICAgICAgICBDcmFzaGVzIGluIFdlYkNvcmU6OlJlcGxh Y2VTZWxlY3Rpb25Db21tYW5kOjpkb0FwcGx5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD02NzY2OAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIGVuY2xvc2luZ0Jsb2NrIG1heSByZXR1cm4gTlVMTCwgYnV0IGl0 cyByZXR1cm4gdmFsdWUgd2FzIG5vdCBjaGVja2VkLiBUaGlzIHBhdGNoIGNoZWNrcyBpdC4KKwor ICAgICAgICAqIGVkaXRpbmcvaW5zZXJ0aW5nL2luc2VydC02NzY2OC1jcmFzaC1leHBlY3RlZC50 eHQ6IEFkZGVkLgorICAgICAgICAqIGVkaXRpbmcvaW5zZXJ0aW5nL2luc2VydC02NzY2OC1jcmFz aC5odG1sOiBBZGRlZC4KKwogMjAxMS0wOS0wNyAgQWRhbSBCYXJ0aCAgPGFiYXJ0aEB3ZWJraXQu b3JnPgogCiAgICAgICAgIFVwZGF0ZSBiYXNlbGluZXMgZm9yIHN2Zy9hcy1iYWNrZ3JvdW5kLWlt YWdlLiAgVGhlc2UgcmVzdWx0cyBhbGwgYXBwZWFyCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9l ZGl0aW5nL2luc2VydGluZy9pbnNlcnQtNjc2NjgtY3Jhc2gtZXhwZWN0ZWQudHh0IGIvTGF5b3V0 VGVzdHMvZWRpdGluZy9pbnNlcnRpbmcvaW5zZXJ0LTY3NjY4LWNyYXNoLWV4cGVjdGVkLnR4dApu ZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwLi5jMDdjNjQxMWQ0MGM5N2Q0YzkyZTg0M2FkMjgxNDU5YWRmNDA3OWY3Ci0tLSAv ZGV2L251bGwKKysrIGIvTGF5b3V0VGVzdHMvZWRpdGluZy9pbnNlcnRpbmcvaW5zZXJ0LTY3NjY4 LWNyYXNoLWV4cGVjdGVkLnR4dApAQCAtMCwwICsxLDMgQEAKK1RoaXMgdGVzdCBlbnN1cmVzIFdl YktpdCBkb2VzIG5vdCBjcmFzaC4KKworUEFTUwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZWRp dGluZy9pbnNlcnRpbmcvaW5zZXJ0LTY3NjY4LWNyYXNoLmh0bWwgYi9MYXlvdXRUZXN0cy9lZGl0 aW5nL2luc2VydGluZy9pbnNlcnQtNjc2NjgtY3Jhc2guaHRtbApuZXcgZmlsZSBtb2RlIDEwMDY0 NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi45YmI1NDUz NjFjZWZmZDA1YzA1ZmM3Zjk1MmQwZTQ5NTg0ODI4ZjQ4Ci0tLSAvZGV2L251bGwKKysrIGIvTGF5 b3V0VGVzdHMvZWRpdGluZy9pbnNlcnRpbmcvaW5zZXJ0LTY3NjY4LWNyYXNoLmh0bWwKQEAgLTAs MCArMSwxMSBAQAorPGZlU3BvdExpZ2h0PjxzdWIgaWQ9ImRpdiIgY29udGVudGVkaXRhYmxlPSJ0 cnVlIj48c2NyaXB0PgoraWYgKHdpbmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikKKyAgICBsYXlv dXRUZXN0Q29udHJvbGxlci5kdW1wQXNUZXh0KCk7CisKK3ZhciBzZWwgPSB3aW5kb3cuZ2V0U2Vs ZWN0aW9uKCk7CisKK3NlbC5zZXRQb3NpdGlvbihkaXYsIDApOworZG9jdW1lbnQuZXhlY0NvbW1h bmQoIkluc2VydEhUTUwiLCBmYWxzZSwgIjxkbD4iKTsKKworZG9jdW1lbnQud3JpdGVsbignVGhp cyB0ZXN0IGVuc3VyZXMgV2ViS2l0IGRvZXMgbm90IGNyYXNoLjxicj48YnI+UEFTUycpOworPC9z Y3JpcHQ+CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cKaW5kZXggYzgzZTc5MTFmYWZkNDljYzk5ZGVjNzZhYTUyYTJjMTNjMTJi OGNlNC4uZmQwYTYxOTliMDMwZGNhNTRkYjRlYTA2ODQ1OGQwZmFmYzVjYzA3NCAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDE3IEBACisyMDExLTA5LTA3ICBTaGlueWEgS2F3YW5ha2EgIDxzaGlueWFr QGdvb2dsZS5jb20+CisKKyAgICAgICAgQ3Jhc2hlcyBpbiBXZWJDb3JlOjpSZXBsYWNlU2VsZWN0 aW9uQ29tbWFuZDo6ZG9BcHBseQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9Njc2NjgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBlbmNsb3NpbmdCbG9jayBtYXkgcmV0dXJuIE5VTEwsIGJ1dCBpdHMgcmV0dXJu IHZhbHVlIHdhcyBub3QgY2hlY2tlZC4gVGhpcyBwYXRjaCBjaGVja3MgaXQuCisKKyAgICAgICAg VGVzdDogZWRpdGluZy9pbnNlcnRpbmcvaW5zZXJ0LTY3NjY4LWNyYXNoLmh0bWwKKworICAgICAg ICAqIGVkaXRpbmcvUmVwbGFjZVNlbGVjdGlvbkNvbW1hbmQuY3BwOgorICAgICAgICAoV2ViQ29y ZTo6UmVwbGFjZVNlbGVjdGlvbkNvbW1hbmQ6OmRvQXBwbHkpOiBBZGRlZCBudWxsIGNoZWNrLgor CiAyMDExLTA5LTA3ICBKdWxpZW4gQ2hhZmZyYWl4ICA8amNoYWZmcmFpeEB3ZWJraXQub3JnPgog CiAgICAgICAgIG9mZnNldEZyb21Sb290IG9wdGltaXphdGlvbiBpcyBkaXNhYmxlZCBhZnRlciBy OTM4MzcKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvUmVwbGFjZVNlbGVjdGlv bkNvbW1hbmQuY3BwIGIvU291cmNlL1dlYkNvcmUvZWRpdGluZy9SZXBsYWNlU2VsZWN0aW9uQ29t bWFuZC5jcHAKaW5kZXggNzY5N2VhZDBmZTkxNGJhYjU5YWYzZmNjYmJiNjg3MGFmYTRhMmRiOS4u NGM1OGY2YzcyNjgyZWY2MDIzYjgwYWQ0MTg3YjVmMWIyNzUzNDhmNCAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvZWRpdGluZy9SZXBsYWNlU2VsZWN0aW9uQ29tbWFuZC5jcHAKKysrIGIvU291 cmNlL1dlYkNvcmUvZWRpdGluZy9SZXBsYWNlU2VsZWN0aW9uQ29tbWFuZC5jcHAKQEAgLTk5NCw3 ICs5OTQsNyBAQCB2b2lkIFJlcGxhY2VTZWxlY3Rpb25Db21tYW5kOjpkb0FwcGx5KCkKIAogICAg IE5vZGUqIGJsb2NrU3RhcnQgPSBlbmNsb3NpbmdCbG9jayhpbnNlcnRpb25Qb3MuZGVwcmVjYXRl ZE5vZGUoKSk7CiAgICAgaWYgKChpc0xpc3RFbGVtZW50KHJlZk5vZGUuZ2V0KCkpIHx8IChpc0xl Z2FjeUFwcGxlU3R5bGVTcGFuKHJlZk5vZGUuZ2V0KCkpICYmIGlzTGlzdEVsZW1lbnQocmVmTm9k ZS0+Zmlyc3RDaGlsZCgpKSkpCi0gICAgICAgICYmIGJsb2NrU3RhcnQtPnJlbmRlcmVyKCktPmlz TGlzdEl0ZW0oKSkKKyAgICAgICAgJiYgYmxvY2tTdGFydCAmJiBibG9ja1N0YXJ0LT5yZW5kZXJl cigpLT5pc0xpc3RJdGVtKCkpCiAgICAgICAgIHJlZk5vZGUgPSBpbnNlcnRBc0xpc3RJdGVtcyhy ZWZOb2RlLCBibG9ja1N0YXJ0LCBpbnNlcnRpb25Qb3MpOwogICAgIGVsc2UKICAgICAgICAgaW5z ZXJ0Tm9kZUF0QW5kVXBkYXRlTm9kZXNJbnNlcnRlZChyZWZOb2RlLCBpbnNlcnRpb25Qb3MpOwo=