Bug 32511 – Dragging an SVG image triggers assertion in DragController::startDrag()

• Component: SVG
• Status: RESOLVED FIXED

[Andreas Kling]

DragController.cpp:675: ASSERT(!image->filenameExtension().isEmpty());

SVGImage doesn't implement filenameExtension(), and the default implementation returns an empty string.

[Andreas Kling]

Created attachment 44788 [details]
Add SVGImage::filenameExtension()

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44788 [details] without any errors.

[Darin Adler]

Comment on attachment 44788 [details]
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.

[Andreas Kling]

Created attachment 45319 [details]
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.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45319 [details] without any errors.

[Andreas Kling]

Minor brain glitch, it's not actually the *same* patch, I made SVGImage::filenameExtension() private as per your suggestion.

[WebKit Commit Bot]

Comment on attachment 45319 [details]
Add SVGImage::filenameExtension() + regression test

Clearing flags on attachment: 45319

Committed r52448: <http://trac.webkit.org/changeset/52448>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.