44153 2010-08-17 22:47:06 -0700 Null dereference in DOMSelection::deleteFromDocument 2012-04-26 12:47:40 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 CLOSED FIXED P2 Normal --- 42959 84991 1 abarth abarth ap commit-queue darin enrica eric kojii rniwa skylined oldest_to_newest 265954 0 abarth 2010-08-17 22:47:06 -0700 Null dereference in DOMSelection::deleteFromDocument 265956 1 64671 abarth 2010-08-17 22:49:35 -0700 Created attachment 64671 Patch 265976 2 64671 commit-queue 2010-08-18 00:38:22 -0700 Comment on attachment 64671 Patch Clearing flags on attachment: 64671 Committed r65587: <http://trac.webkit.org/changeset/65587> 265977 3 commit-queue 2010-08-18 00:38:26 -0700 All reviewed patches have been landed. Closing bug. 266247 4 darin 2010-08-18 10:55:17 -0700 It's unfortunate we are landing this without a test case. I have run into similar circumstances many times before with fuzzers, crashes I understood fully but could not reproduce with a test case, and in the past I have typically struggled to make a test case for a day or two before landing without a test. 266326 5 abarth 2010-08-18 12:56:42 -0700 We can revert the change if you like. I have very little understanding of layout or editing. I suspect that someone who understands these areas might be able to create a test case. 266363 6 darin 2010-08-18 14:05:56 -0700 (In reply to comment #5) > We can revert the change if you like. I have very little understanding of layout or editing. I suspect that someone who understands these areas might be able to create a test case. I don’t have a specific strategy to suggest to make sure a test case gets created. I don’t care who creates it. I would like to see someone add it to WebKit, though. 266390 7 abarth 2010-08-18 14:46:38 -0700 Reopened for the test. @enrica: any thoughts on who might understand this code well enough to write a test for this issue? 267060 8 ap 2010-08-19 16:50:53 -0700 Adam, do you have a stack trace from this preserved somewhere? 267064 9 abarth 2010-08-19 16:53:34 -0700 I didn't save the stack. It was pretty boring. This method is callable directly from JavaScript. The trick is to get the DOM / render tree set up in such a way that updateLayout() makes the current selection disappear. There's likely to be some trick to get that it happen, but I don't know enough about layout or selections to know how to set that up. 267066 10 ap 2010-08-19 16:55:24 -0700 I don't know a lot about it either, but my guess is that one adds display:none to a style. 267940 11 enrica 2010-08-23 09:09:43 -0700 One possible way to test this is: 1. create a selection 2. execCommand('indent'). This will move the selection under a block by cloning the selection under the new block and deleting the original one 3. call deleteFromDocument on the original selection 270595 12 abarth 2010-08-27 11:04:38 -0700 That doesn't seem to work. We need to make this condition true and have layout() blow away the selection. if (v && renderer() && (v->layoutPending() || renderer()->needsLayout())) v->layout(); 270596 13 65734 abarth 2010-08-27 11:05:21 -0700 Created attachment 65734 Failed attempt at a test 283938 14 ap 2010-09-23 14:13:57 -0700 I got this crash once, and added debug output to DOMSelection::deleteFromDocument(). If I hit this again, it will be easy to construct a test case. But it's quite rare. 286412 15 eric 2010-09-28 11:47:44 -0700 I'm not sure this should be open any longer. Our window for creating a test is gone. Either this should be reverted or closed it seems. 286419 16 ap 2010-09-28 11:53:23 -0700 Sad. Unfortunately, I never got this crash for the second time - but considering how easy it was to dump current selection state before it was lost for someone who could reproduce this, it's really wrong to not have a regression test. 286522 17 69103 rniwa 2010-09-28 14:14:41 -0700 Created attachment 69103 failed attempt, hits other crashes This is a hard one to reproduce. I hit all sorts of other crashes before getting to this one. Try replacing 'bold' by 'indent', 'insertorderedlist', etc... 286532 18 eric 2010-09-28 14:26:37 -0700 (In reply to comment #17) > Created an attachment (id=69103) [details] > failed attempt, hits other crashes > > This is a hard one to reproduce. I hit all sorts of other crashes before getting to this one. Try replacing 'bold' by 'indent', 'insertorderedlist', etc... Please file bugs about the other crashes! :) Those should be fixed too... 286542 19 rniwa 2010-09-28 14:42:41 -0700 (In reply to comment #18) > (In reply to comment #17) > > Created an attachment (id=69103) [details] [details] > > failed attempt, hits other crashes > > > > This is a hard one to reproduce. I hit all sorts of other crashes before getting to this one. Try replacing 'bold' by 'indent', 'insertorderedlist', etc... > > Please file bugs about the other crashes! :) Those should be fixed too... Sure I can file bugs. But the real issue with these kind of crashes is our making a lot of assumptions after calling functions that modify DOM such as appendChild. We ultimately need to make DOM mutation events asynchronous and don't fire it until we get out of composite editing commands. Because it's really hard (or possibly impossible) for us to handle all possible modifications script can make at every stage of editing code. I'm hoping to send out a proposal about this in a week or two. 610665 20 kojii 2012-04-26 09:02:22 -0700 In case you're still looking for a test case, I happened to come to this bug from bug 42844 and it looks like it has a reproducible script attached. 610825 21 rniwa 2012-04-26 12:32:54 -0700 *** Bug 42844 has been marked as a duplicate of this bug. *** 610828 22 rniwa 2012-04-26 12:34:30 -0700 Let's add a test for this. 64671 2010-08-17 22:49:35 -0700 2010-08-18 00:38:22 -0700 Patch bug-44153-20100817224933.patch text/plain 1721 abarth ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ODc2ZDFlNTQwNTI0OGM2NTFkYjllNTM0Nzc4MDFjZjE5NTI3NDk5My4uNTVmZDI2MzVkY2I4NDg0 ZTU3NjM5ZWY3MDY1MDViYzQ3MGFlNzhmMSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTIsNiArMiwyMyBAQAogCiAgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgogCisgICAgICAgIE51bGwgZGVyZWZlcmVuY2UgaW4gRE9N U2VsZWN0aW9uOjpkZWxldGVGcm9tRG9jdW1lbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTQ0MTUzCisKKyAgICAgICAgZGVsZXRlRnJvbURvY3VtZW50 IGNoZWNrcyBzZWxlY3Rpb24tPmlzTm9uZSgpIGJlZm9yZSBjYWxsaW5nCisgICAgICAgIHNlbGVj dGlvbi0+c2VsZWN0aW9uKCkudG9Ob3JtYWxpemVkUmFuZ2UoKSwgYnV0IHRvTm9ybWFsaXplZFJh bmdlKCkKKyAgICAgICAgbm90ZXMgdGhhdCBpdCBuZWVkcyB0byB1cGRhdGVMYXlvdXQoKSwgd2hp Y2ggY2FuIG1ha2UgdGhlIHNlbGVjdGlvbgorICAgICAgICBpc05vbmUoKSBhZ2Fpbi4gIEluIHRo YXQgY2FzZSwgd2UgY3Jhc2ggb24gYSBOVUxMIHBvaW50ZXIgaW4KKyAgICAgICAgZGVsZXRlRnJv bURvY3VtZW50LiAgSSBkb24ndCBrbm93IGhvdyB0byB0cmlnZ2VyIHRoYXQgc2l0dWF0aW9uIGlu IGEKKyAgICAgICAgdGVzdCwgYnV0IGNyb3NzX2Z1enogd2FzIGFibGUgdG8gaGl0IGl0LCBzbyB3 ZSBzaG91bGQgZml4IGl0LgorCisgICAgICAgICogcGFnZS9ET01TZWxlY3Rpb24uY3BwOgorICAg ICAgICAoV2ViQ29yZTo6RE9NU2VsZWN0aW9uOjpkZWxldGVGcm9tRG9jdW1lbnQpOgorCisyMDEw LTA4LTE3ICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKICAgICAgICAgTk9UX1JFQUNIRUQgaXMgcmVhY2hhYmxl IGluIFNWR0xlbmd0aAogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NDQxNTAKIApkaWZmIC0tZ2l0IGEvV2ViQ29yZS9wYWdlL0RPTVNlbGVjdGlvbi5jcHAg Yi9XZWJDb3JlL3BhZ2UvRE9NU2VsZWN0aW9uLmNwcAppbmRleCBkNjIwNTFlMDQwMDY0NTY4MjQ1 OGUwZGEwNjIyYzgwNDgwNTU4ZDdlLi4xMDZkZDEzNjBkNmE4YWYyZmY0NWViN2YwNTA4MGM1Y2Q4 OTFlODY3IDEwMDY0NAotLS0gYS9XZWJDb3JlL3BhZ2UvRE9NU2VsZWN0aW9uLmNwcAorKysgYi9X ZWJDb3JlL3BhZ2UvRE9NU2VsZWN0aW9uLmNwcApAQCAtNDI3LDYgKzQyNyw4IEBAIHZvaWQgRE9N U2VsZWN0aW9uOjpkZWxldGVGcm9tRG9jdW1lbnQoKQogICAgICAgICBzZWxlY3Rpb24tPm1vZGlm eShTZWxlY3Rpb25Db250cm9sbGVyOjpBbHRlcmF0aW9uRXh0ZW5kLCBTZWxlY3Rpb25Db250cm9s bGVyOjpEaXJlY3Rpb25CYWNrd2FyZCwgQ2hhcmFjdGVyR3JhbnVsYXJpdHkpOwogCiAgICAgUmVm UHRyPFJhbmdlPiBzZWxlY3RlZFJhbmdlID0gc2VsZWN0aW9uLT5zZWxlY3Rpb24oKS50b05vcm1h bGl6ZWRSYW5nZSgpOworICAgIGlmICghc2VsZWN0ZWRSYW5nZSkKKyAgICAgICAgcmV0dXJuOwog CiAgICAgRXhjZXB0aW9uQ29kZSBlYyA9IDA7CiAgICAgc2VsZWN0ZWRSYW5nZS0+ZGVsZXRlQ29u dGVudHMoZWMpOwo= 65734 2010-08-27 11:05:21 -0700 2010-08-27 11:05:21 -0700 Failed attempt at a test deleteFromDocument-crash.html text/html 630 abarth PGh0bWw+Cjxib2R5Pgo8ZGl2IGlkPSJ0ZXN0Ij5mb29iYXJiYXo8L2Rpdj4KPGRpdiBpZD0iY29u c29sZSI+PC9kaXY+CjxzY3JpcHQ+CmlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIpCiAg ICBsYXlvdXRUZXN0Q29udHJvbGxlci5kdW1wQXNUZXh0KCk7Cgp2YXIgciA9IGRvY3VtZW50LmNy ZWF0ZVJhbmdlKCk7CnZhciBzID0gd2luZG93LmdldFNlbGVjdGlvbigpOwp2YXIgdGVzdERpdiA9 IGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJ0ZXN0Iik7CgpmdW5jdGlvbiBsb2coc3RyKSB7CiAg ICB2YXIgbGkgPSBkb2N1bWVudC5jcmVhdGVFbGVtZW50KCJsaSIpOwogICAgbGkuYXBwZW5kQ2hp bGQoZG9jdW1lbnQuY3JlYXRlVGV4dE5vZGUoc3RyKSk7CiAgICBkb2N1bWVudC5nZXRFbGVtZW50 QnlJZCgiY29uc29sZSIpLmFwcGVuZENoaWxkKGxpKTsKfQoKdmFyIHIgPSBkb2N1bWVudC5jcmVh dGVSYW5nZSgpOwpyLnNldFN0YXJ0KHRlc3REaXYuZmlyc3RDaGlsZCwgMSk7CnIuc2V0RW5kKHRl c3REaXYuZmlyc3RDaGlsZCwgMyk7CnMuYWRkUmFuZ2Uocik7CmRvY3VtZW50LmV4ZWNDb21tYW5k KCdpbmRlbnQnKQpzLmRlbGV0ZUZyb21Eb2N1bWVudCgpCjwvc2NyaXB0Pgo8L2JvZHk+CjwvaHRt bD4K 69103 2010-09-28 14:14:41 -0700 2010-09-28 14:14:41 -0700 failed attempt, hits other crashes selection-crash.html text/html 536 rniwa PGh0bWw+CjxkaXYgaWQ9InRlc3QiIGNvbnRlbnRlZGl0YWJsZT5oZWxsbyB3b3JsZDwvZGl2Pgo8 c2NyaXB0PgoKdmFyIHRlc3QgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgndGVzdCcpOwp3aW5k b3cuZ2V0U2VsZWN0aW9uKCkuc2V0UG9zaXRpb24odGVzdCwgMCk7CndpbmRvdy5nZXRTZWxlY3Rp b24oKS5tb2RpZnkoJ21vdmUnLCAnZm9yd2FyZCcsICdjaGFyYWN0ZXInKTsKd2luZG93LmdldFNl bGVjdGlvbigpLm1vZGlmeSgnZXh0ZW5kJywgJ2ZvcndhcmQnLCAnY2hhcmFjdGVyJyk7CgpmdW5j dGlvbiBtb2RpZmllZCgpIHsKICAgIHdoaWxlICh0ZXN0LmZpcnN0Q2hpbGQpCiAgICAgICAgdGVz dC5yZW1vdmVDaGlsZCh0ZXN0LmZpcnN0Q2hpbGQpOwogICAgd2luZG93LmdldFNlbGVjdGlvbigp LmRlbGV0ZUZyb21Eb2N1bWVudCgpOwp9Cgp0ZXN0LmFkZEV2ZW50TGlzdGVuZXIoJ0RPTVN1YnRy ZWVNb2RpZmllZCcsIG1vZGlmaWVkLCBmYWxzZSk7CmRvY3VtZW50LmV4ZWNDb21tYW5kKCdib2xk Jyk7Cgo8L3NjcmlwdD4KPC9odG1sPgo=