32511 2009-12-14 05:34:37 -0800 Dragging an SVG image triggers assertion in DragController::startDrag() 2009-12-21 10:58:36 -0800 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 kling webkit-unassigned commit-queue kling webkit.review.bot oldest_to_newest 171353 0 kling 2009-12-14 05:34:37 -0800 DragController.cpp:675: ASSERT(!image->filenameExtension().isEmpty()); SVGImage doesn't implement filenameExtension(), and the default implementation returns an empty string. 171355 1 44788 kling 2009-12-14 05:40:49 -0800 Created attachment 44788 Add SVGImage::filenameExtension() 171356 2 webkit.review.bot 2009-12-14 05:41:48 -0800 style-queue ran check-webkit-style on attachment 44788 without any errors. 171446 3 44788 darin 2009-12-14 09:45:47 -0800 Comment on attachment 44788 Add SVGImage::filenameExtension() Thanks, looks like the right fix. > + virtual String filenameExtension() const; Virtual functions like this one normally should be private rather than public. It's a programming error to call this function on a pointer known to be an SVGImage* so it's good to get a compiler error in that case. And a private member function still does just fine at overriding the base class. Can we make a regression test for this? Normally we require regression tests for all bug fixes, and we have quite a few dragging tests already, so it can probably be done. I'm going to say review-, but if you try and find we can't make a regression test then revise the ChangeLog to explain why, and then feel free to put this up for review again. 173936 4 45319 kling 2009-12-21 03:52:02 -0800 Created attachment 45319 Add SVGImage::filenameExtension() + regression test Thanks for your comments! Here's the same patch with an added regression test. Please note that the test will only fail when compiled in debug mode, since the problem is a failing assertion. 173937 5 webkit.review.bot 2009-12-21 03:55:56 -0800 style-queue ran check-webkit-style on attachment 45319 without any errors. 173954 6 kling 2009-12-21 05:09:00 -0800 Minor brain glitch, it's not actually the *same* patch, I made SVGImage::filenameExtension() private as per your suggestion. 174007 7 45319 commit-queue 2009-12-21 10:58:31 -0800 Comment on attachment 45319 Add SVGImage::filenameExtension() + regression test Clearing flags on attachment: 45319 Committed r52448: <http://trac.webkit.org/changeset/52448> 174008 8 commit-queue 2009-12-21 10:58:36 -0800 All reviewed patches have been landed. Closing bug. 44788 2009-12-14 05:40:49 -0800 2009-12-21 03:52:02 -0800 Add SVGImage::filenameExtension() bug-32511.diff text/plain 1360 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NTA0NDk4Yi4uZTc2ZTkwYiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAwOS0xMi0xNCAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEZpeCBhc3NlcnRpb24gZmFpbHVyZSB3aGVuIGRyYWdnaW5n IGFuIFNWRyBpbWFnZS4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MzI1MTEKKworICAgICAgICAqIHN2Zy9ncmFwaGljcy9TVkdJbWFnZS5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpTVkdJbWFnZTo6ZmlsZW5hbWVFeHRlbnNpb24pOgorICAgICAgICAq IHN2Zy9ncmFwaGljcy9TVkdJbWFnZS5oOgorCiAyMDA5LTEyLTAzICBIb2xnZXIgSGFucyBQZXRl ciBGcmV5dGhlciAgPHplY2tlQHNlbGZpc2gub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNp bW9uIEhhdXNtYW5uLgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvU1ZHSW1hZ2Uu Y3BwIGIvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvU1ZHSW1hZ2UuY3BwCmluZGV4IDk3Njc4NmEuLmE1 MWZlM2MgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvc3ZnL2dyYXBoaWNzL1NWR0ltYWdlLmNwcAorKysg Yi9XZWJDb3JlL3N2Zy9ncmFwaGljcy9TVkdJbWFnZS5jcHAKQEAgLTI2Nyw2ICsyNjcsMTEgQEAg Ym9vbCBTVkdJbWFnZTo6ZGF0YUNoYW5nZWQoYm9vbCBhbGxEYXRhUmVjZWl2ZWQpCiAgICAgcmV0 dXJuIG1fcGFnZTsKIH0KIAorU3RyaW5nIFNWR0ltYWdlOjpmaWxlbmFtZUV4dGVuc2lvbigpIGNv bnN0Cit7CisgICAgcmV0dXJuICJzdmciOworfQorCiB9CiAKICNlbmRpZiAvLyBFTkFCTEUoU1ZH KQpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvU1ZHSW1hZ2UuaCBiL1dlYkNvcmUv c3ZnL2dyYXBoaWNzL1NWR0ltYWdlLmgKaW5kZXggMTBmMzliYS4uOTViYzM1ZiAxMDA2NDQKLS0t IGEvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvU1ZHSW1hZ2UuaAorKysgYi9XZWJDb3JlL3N2Zy9ncmFw aGljcy9TVkdJbWFnZS5oCkBAIC00NCw2ICs0NCw4IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKICAg ICAgICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgU1ZHSW1hZ2Uob2JzZXJ2ZXIpKTsKICAgICAg ICAgfQogCisgICAgICAgIHZpcnR1YWwgU3RyaW5nIGZpbGVuYW1lRXh0ZW5zaW9uKCkgY29uc3Q7 CisKICAgICBwcml2YXRlOgogICAgICAgICB2aXJ0dWFsIH5TVkdJbWFnZSgpOwogCg== 45319 2009-12-21 03:52:02 -0800 2009-12-21 10:58:31 -0800 Add SVGImage::filenameExtension() + regression test bug-32511-2.diff text/plain 4146 kling ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCAxYjBhMzU1Li5jM2I5MmI1IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDktMTIt MjEgIEFuZHJlYXMgS2xpbmcgIDxhbmRyZWFzLmtsaW5nQG5va2lhLmNvbT4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUZXN0IHRoYXQgZHJhZ2dpbmcg YW4gU1ZHIGFzIDxpbWc+IGRvZXNuJ3QgY3Jhc2ggdGhlIGJyb3dzZXIuCisgICAgICAgIE5vdGUg dGhhdCB0aGlzIG9ubHkgd29ya3MgaW4gZGVidWcgbW9kZSBhcyBpdCdzIGEgZmFpbGluZyBhc3Nl cnQuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMy NTExCisKKyAgICAgICAgKiBmYXN0L2ltYWdlcy9kcmFnLXN2Zy1hcy1pbWFnZS5odG1sOiBBZGRl ZC4KKyAgICAgICAgKiBwbGF0Zm9ybS9xdC9Ta2lwcGVkOiBmYXN0L2ltYWdlcy9kcmFnLXN2Zy1h cy1pbWFnZS5odG1sIGFkZGVkLgorCiAyMDA5LTEyLTIwICBLZW50IFRhbXVyYSAgPHRrZW50QGNo cm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBBZGxlci4KZGlmZiAtLWdp dCBhL0xheW91dFRlc3RzL2Zhc3QvaW1hZ2VzL2RyYWctc3ZnLWFzLWltYWdlLWV4cGVjdGVkLnR4 dCBiL0xheW91dFRlc3RzL2Zhc3QvaW1hZ2VzL2RyYWctc3ZnLWFzLWltYWdlLWV4cGVjdGVkLnR4 dApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi40ZmZlNGViCi0tLSAvZGV2L251 bGwKKysrIGIvTGF5b3V0VGVzdHMvZmFzdC9pbWFnZXMvZHJhZy1zdmctYXMtaW1hZ2UtZXhwZWN0 ZWQudHh0CkBAIC0wLDAgKzEsMiBAQAorCitUaGlzIHRlc3QgY2hlY2tzIHRoYXQgZHJhZ2dpbmcg YW4gU1ZHIGltYWdlIGRvZXNuJ3QgY3Jhc2ggdGhlIGJyb3dzZXIuCmRpZmYgLS1naXQgYS9MYXlv dXRUZXN0cy9mYXN0L2ltYWdlcy9kcmFnLXN2Zy1hcy1pbWFnZS5odG1sIGIvTGF5b3V0VGVzdHMv ZmFzdC9pbWFnZXMvZHJhZy1zdmctYXMtaW1hZ2UuaHRtbApuZXcgZmlsZSBtb2RlIDEwMDY0NApp bmRleCAwMDAwMDAwLi45ZDQ3NzQ5Ci0tLSAvZGV2L251bGwKKysrIGIvTGF5b3V0VGVzdHMvZmFz dC9pbWFnZXMvZHJhZy1zdmctYXMtaW1hZ2UuaHRtbApAQCAtMCwwICsxLDMwIEBACis8aHRtbD4K KzxoZWFkPgorPHNjcmlwdCB0eXBlPSJ0ZXh0L2phdmFzY3JpcHQiPgorZnVuY3Rpb24gdGVzdCgp IHsKKworICAgIGlmICghd2luZG93LmxheW91dFRlc3RDb250cm9sbGVyIHx8ICF3aW5kb3cuZXZl bnRTZW5kZXIpCisgICAgICAgIHJldHVybjsKKworICAgIGxheW91dFRlc3RDb250cm9sbGVyLmR1 bXBBc1RleHQoKTsKKworICAgIHZhciBkcmFnTWUgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgi ZHJhZ01lIik7CisgICAgdmFyIHN0YXJ0WCA9IGRyYWdNZS5vZmZzZXRMZWZ0ICsgKGRyYWdNZS5v ZmZzZXRXaWR0aCAvIDIpOworICAgIHZhciBzdGFydFkgPSBkcmFnTWUub2Zmc2V0VG9wICsgKGRy YWdNZS5vZmZzZXRUb3AgLyAyKTsKKyAgICB2YXIgZW5kWCA9IHN0YXJ0WCArIDEwMDsKKyAgICB2 YXIgZW5kWSA9IHN0YXJ0WSArIDEwMDsKKworICAgIGV2ZW50U2VuZGVyLm1vdXNlTW92ZVRvKHN0 YXJ0WCwgc3RhcnRZKTsKKyAgICBldmVudFNlbmRlci5tb3VzZURvd24oKTsKKyAgICBldmVudFNl bmRlci5tb3VzZU1vdmVUbyhlbmRYLCBlbmRZKTsKKyAgICBldmVudFNlbmRlci5tb3VzZVVwKCk7 Cit9Cis8L3NjcmlwdD4KKzwvaGVhZD4KKzxib2R5IG9ubG9hZD0idGVzdCgpIj4KKyAgICA8aW1n IGlkPSJkcmFnTWUiIHNyYz0ncmVzb3VyY2VzL2dyZWVuLWZpeGVkLXNpemUtcmVjdC5zdmcnPgor ICAgIDxkaXY+CisgICAgICAgIFRoaXMgdGVzdCBjaGVja3MgdGhhdCBkcmFnZ2luZyBhbiBTVkcg aW1hZ2UgZG9lc24ndCBjcmFzaCB0aGUgYnJvd3Nlci4KKyAgICA8L2Rpdj4KKzwvYm9keT4KKzwv aHRtbD4KZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQgYi9MYXlv dXRUZXN0cy9wbGF0Zm9ybS9xdC9Ta2lwcGVkCmluZGV4IGIzZTM1ZTYuLmMxMzQ5MzkgMTAwNjQ0 Ci0tLSBhL0xheW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQKKysrIGIvTGF5b3V0VGVzdHMv cGxhdGZvcm0vcXQvU2tpcHBlZApAQCAtMjQ5LDYgKzI0OSw5IEBAIGZhc3QvZm9ybXMvaW5wdXQt ZmlsZS1yZS1yZW5kZXIuaHRtbAogZmFzdC9ldmVudHMvZHJhZy1maWxlLWNyYXNoLmh0bWwKIGh0 dHAvdGVzdHMvc2VjdXJpdHkvY2xpcGJvYXJkL2NsaXBib2FyZC1maWxlLWFjY2Vzcy5odG1sCiAK KyMgTWlzc2luZyBkcmFnICYgZHJvcCBmdW5jdGlvbmFsaXR5IGluIERSVAorZmFzdC9pbWFnZXMv ZHJhZy1zdmctYXMtaW1hZ2UuaHRtbAorCiAjIC0tLS0tLS0gbWlzc2luZyBldmVudFNlbmRlci56 b29tUGFnZUluCiBmYXN0L2V2ZW50cy9jbGllbnRYWS1pbi16b29tLWFuZC1zY3JvbGwuaHRtbAog ZmFzdC9kb20vRG9jdW1lbnQvQ2FyZXRSYW5nZUZyb21Qb2ludC9jYXJldFJhbmdlRnJvbVBvaW50 LWluLXpvb20tYW5kLXNjcm9sbC5odG1sCmRpZmYgLS1naXQgYS9XZWJDb3JlL0NoYW5nZUxvZyBi L1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IGQyNjNlZjYuLjhjNjg3MDcgMTAwNjQ0Ci0tLSBhL1dl YkNvcmUvQ2hhbmdlTG9nCisrKyBiL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAK KzIwMDktMTItMjEgIEFuZHJlYXMgS2xpbmcgIDxhbmRyZWFzLmtsaW5nQG5va2lhLmNvbT4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBGaXggYXNzZXJ0 aW9uIGZhaWx1cmUgd2hlbiBkcmFnZ2luZyBhbiBTVkcgaW1hZ2UuCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMjUxMQorCisgICAgICAgIFRlc3Q6IGZh c3QvaW1hZ2VzL2RyYWctc3ZnLWFzLWltYWdlLmh0bWwKKworICAgICAgICAqIHN2Zy9ncmFwaGlj cy9TVkdJbWFnZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTVkdJbWFnZTo6ZmlsZW5hbWVFeHRl bnNpb24pOiBSZXR1cm4gInN2ZyIKKyAgICAgICAgKiBzdmcvZ3JhcGhpY3MvU1ZHSW1hZ2UuaDoK KwogMjAwOS0xMi0xOCAgSnVsaWVuIENoYWZmcmFpeCAgPGpjaGFmZnJhaXhAcGxleW8uY29tPgog CiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9z dmcvZ3JhcGhpY3MvU1ZHSW1hZ2UuY3BwIGIvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvU1ZHSW1hZ2Uu Y3BwCmluZGV4IGY5YzUyNzcuLjA1MGRjY2EgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvc3ZnL2dyYXBo aWNzL1NWR0ltYWdlLmNwcAorKysgYi9XZWJDb3JlL3N2Zy9ncmFwaGljcy9TVkdJbWFnZS5jcHAK QEAgLTI2OCw2ICsyNjgsMTEgQEAgYm9vbCBTVkdJbWFnZTo6ZGF0YUNoYW5nZWQoYm9vbCBhbGxE YXRhUmVjZWl2ZWQpCiAgICAgcmV0dXJuIG1fcGFnZTsKIH0KIAorU3RyaW5nIFNWR0ltYWdlOjpm aWxlbmFtZUV4dGVuc2lvbigpIGNvbnN0Cit7CisgICAgcmV0dXJuICJzdmciOworfQorCiB9CiAK ICNlbmRpZiAvLyBFTkFCTEUoU1ZHKQpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9zdmcvZ3JhcGhpY3Mv U1ZHSW1hZ2UuaCBiL1dlYkNvcmUvc3ZnL2dyYXBoaWNzL1NWR0ltYWdlLmgKaW5kZXggMTBmMzli YS4uYTA0MTRmZSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvU1ZHSW1hZ2UuaAor KysgYi9XZWJDb3JlL3N2Zy9ncmFwaGljcy9TVkdJbWFnZS5oCkBAIC00Nyw2ICs0Nyw4IEBAIG5h bWVzcGFjZSBXZWJDb3JlIHsKICAgICBwcml2YXRlOgogICAgICAgICB2aXJ0dWFsIH5TVkdJbWFn ZSgpOwogCisgICAgICAgIHZpcnR1YWwgU3RyaW5nIGZpbGVuYW1lRXh0ZW5zaW9uKCkgY29uc3Q7 CisKICAgICAgICAgdmlydHVhbCB2b2lkIHNldENvbnRhaW5lclNpemUoY29uc3QgSW50U2l6ZSYp OwogICAgICAgICB2aXJ0dWFsIGJvb2wgdXNlc0NvbnRhaW5lclNpemUoKSBjb25zdDsKICAgICAg ICAgdmlydHVhbCBib29sIGhhc1JlbGF0aXZlV2lkdGgoKSBjb25zdDsK